iio: light: isl29018: overflow/precision fix + cover-glass gain via DT
Subsystems (2)
v3 cover iio: light: isl29018: overflow/precision fix + cover-glass gain via DT
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
Signature · 1 line
2.43.0
PATCH v3 1/3 iio: light: isl29018: fix overflow and precision 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.
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.
--- drivers/iio/light/isl29018.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-)
drivers/iio/light/isl29018.c +11 -5
@@ -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 ; }
Signature · 1 line
2.43.0
PATCH v3 2/3 dt-bindings: iio: light: isl29018: add isil,cover-comp-gain
In reply to Herman van HazendonkDocument 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.
--- .../devicetree/bindings/iio/light/isl29018.yaml | 13 +++++++++++++ 1 file changed, 13 insertions(+)
Documentation/devicetree/bindings/iio/light/isl29018.yaml +13 -0
@@ -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
Signature · 1 line
2.43.0
PATCH v3 3/3 iio: light: isl29018: support cover-glass gain compensation via DT
In reply to Herman van HazendonkBoards 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.
--- drivers/iio/light/isl29018.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-)
drivers/iio/light/isl29018.c +13 -2
@@ -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;
Signature · 1 line
2.43.0
Re: [PATCH v3 2/3] dt-bindings: iio: light: isl29018: add isil,cover-comp-gain
In reply to Herman van HazendonkQuoted 29 lines · 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 <[email protected]>--- .../devicetree/bindings/iio/light/isl29018.yaml | 13 +++++++++++++ 1 file changed, 13 insertions(+)Documentation/devicetree/bindings/iio/light/isl29018.yaml +6
-0@@ -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
Documentation/devicetree/bindings/iio/light/isl29018.yaml properties: · line 44
Quoted 2 lines
Documentation/devicetree/bindings/iio/light/isl29018.yaml +2
-0+ in_illuminance0_calibscale, so it can still be retuned at + runtime through sysfs.
Delete this, driver implementation stuff isn't relevant to the devicetree binding.
With that gone,
pw-bot: changes-requested
Cheers,
Conor.
Quoted 11 lines
Documentation/devicetree/bindings/iio/light/isl29018.yaml +5
-0+ $ref: /schemas/types.yaml#/definitions/uint32 + minimum: 1 + maximum: 65535 + default: 1 + required: - compatible - regSignature · 1 line
2.43.0
Re: [PATCH v3 1/3] iio: light: isl29018: fix overflow and precision in isl29018_read_lux()
In reply to Herman van HazendonkQuoted 29 lines · 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 beyondUINT_MAX. The value wraps before the /1000000 division can save it, producing a wildly wrongdata_x_range.2.
data_x_range*chip->calibscale— even after a correctdata_x_range, multiplying by a calibscale of a few hundred (reasonable for a deeply tinted cover glass) pushes the product pastINT_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 adata_x_rangeof 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_uldivmodon 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 toINT_MAXbefore storing it in the signed int *lux out parameter.
...
drivers/iio/light/isl29018.c static int isl29018_read_sensor_input(struct isl29018_chip *chip, int mode) · line 198
Quoted 3 lines
drivers/iio/light/isl29018.c +2
-1-unsigned int data_x_range; +u32 uscale_rem; +u64 uscale_term, data_x_range, result;
Please, try to keep reversed xmas tree order.
...
drivers/iio/light/isl29018.c static int isl29018_read_sensor_input(struct isl29018_chip *chip, int mode) · line 212
Quoted 1 line
drivers/iio/light/isl29018.c +1
-0+ *lux = (int )min_t (u64, result,INT_MAX );
Too many castings. What you most likely want here is clamp(), try never use min_t().
Signature · 2 lines
With Best Regards, Andy Shevchenko
Re: [PATCH v3 3/3] iio: light: isl29018: support cover-glass gain compensation via DT
In reply to Herman van HazendonkQuoted 14 lines · 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_calibscalesysfs 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.
...
drivers/iio/light/isl29018.c static int isl29018_probe(struct i2c_client *client)
Quoted 9 lines
drivers/iio/light/isl29018.c +2
-0struct iio_dev *indio_dev;const void *ddata =NULL ;const char *name; +struct device *dev;int dev_id;int err; + dev = &client->dev;
Make this go together with the definition.
struct device *dev = &client->dev;
...
Use something like
const char *propname; ... propname = "isil,cover-comp-gain";
drivers/iio/light/isl29018.c static int isl29018_probe(struct i2c_client *client) · line 745
Quoted 6 lines
drivers/iio/light/isl29018.c +6
-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" );
And make these three one-liners.
Quoted 3 lines
drivers/iio/light/isl29018.c +3
-0+ }else { + chip->calibscale =1 ; + }
Signature · 2 lines
With Best Regards, Andy Shevchenko
Jonathan Cameron
maintainerRe: [PATCH v3 2/3] dt-bindings: iio: light: isl29018: add isil,cover-comp-gain
In reply to Conor DooleyQuoted 22 lines · On Thu, 4 Jun 2026 18:01:08 +0100 Conor Dooley <[email protected]> wrote:
Quoted 21 lines · 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 <[email protected]>--- .../devicetree/bindings/iio/light/isl29018.yaml | 13 +++++++++++++ 1 file changed, 13 insertions(+)Documentation/devicetree/bindings/iio/light/isl29018.yaml +0
-0@@ -34,6 +34,19 @@ properties: vcc-supply: description: Regulator that provides power to the sensor
Quoted 6 lines
Documentation/devicetree/bindings/iio/light/isl29018.yaml properties:
Quoted 6 lines
Documentation/devicetree/bindings/iio/light/isl29018.yaml +6
-0+ 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.
Is it useful to support decimal points on these values? The userspace interface does and you mention the 'right' answer might be only a few which means precision at that range will be terrible - less of an issue if 100s!
Thanks
Jonathan
Quoted 27 lines
Documentation/devicetree/bindings/iio/light/isl29018.yaml properties: · line 44
Quoted 4 lines
The value seeds
Documentation/devicetree/bindings/iio/light/isl29018.yaml +2
-0+ in_illuminance0_calibscale, so it can still be retuned at + runtime through sysfs.Delete this, driver implementation stuff isn't relevant to the devicetree binding.
With that gone,
Acked-by Conor Dooley <[email protected]>pw-bot: changes-requested
Cheers,
Conor.Quoted 11 lines
Documentation/devicetree/bindings/iio/light/isl29018.yaml +5
-0+ $ref: /schemas/types.yaml#/definitions/uint32 + minimum: 1 + maximum: 65535 + default: 1 + required: - compatible - regSignature · 1 line
2.43.0
Re: [PATCH v3 2/3] dt-bindings: iio: light: isl29018: add isil,cover-comp-gain
Quoted 36 lines · Jonathan Cameron
Quoted 22 lines · On Thu, 4 Jun 2026 18:01:08 +0100 Conor Dooley <[email protected]> wrote:
Quoted 21 lines · 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 <[email protected]>--- .../devicetree/bindings/iio/light/isl29018.yaml | 13 +++++++++++++ 1 file changed, 13 insertions(+)Documentation/devicetree/bindings/iio/light/isl29018.yaml +0
-0@@ -34,6 +34,19 @@ properties: vcc-supply: description: Regulator that provides power to the sensorQuoted 6 lines
Documentation/devicetree/bindings/iio/light/isl29018.yaml properties:
Quoted 6 lines
Documentation/devicetree/bindings/iio/light/isl29018.yaml +6
-0+ 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.Is it useful to support decimal points on these values? The userspace interface does and you mention the 'right' answer might be only a few which means precision at that range will be terrible - less of an issue if 100s!
Quoted 4 lines
Thanks
Jonathan
Hard to say, my old HP TouchPad needs 100 as a value here (taken from legacy 2.6.35 kernel and binaries). So we probably don't need precision, but I have no other references to substantiate.
Thanks
Herman
Quoted 29 lines
Quoted 29 lines
Documentation/devicetree/bindings/iio/light/isl29018.yaml properties: · line 44
Quoted 6 lines
The value seeds
Documentation/devicetree/bindings/iio/light/isl29018.yaml +2
-0+ in_illuminance0_calibscale, so it can still be retuned at + runtime through sysfs.Delete this, driver implementation stuff isn't relevant to the devicetree binding.
With that gone,
Acked-by Conor Dooley <[email protected]>pw-bot: changes-requested
Cheers,
Conor.Quoted 11 lines
Documentation/devicetree/bindings/iio/light/isl29018.yaml +5
-1+ $ref: /schemas/types.yaml#/definitions/uint32 + minimum: 1 + maximum: 65535 + default: 1 + required: - compatible - reg --2.43.0
Re: [PATCH v3 2/3] dt-bindings: iio: light: isl29018: add isil,cover-comp-gain
Quoted 39 lines · On 2026-06-05 15:18, [email protected] wrote:
Quoted 38 lines · Jonathan Cameron
Quoted 22 lines · On Thu, 4 Jun 2026 18:01:08 +0100 Conor Dooley <[email protected]> wrote:
Quoted 21 lines · 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 <[email protected]>--- .../devicetree/bindings/iio/light/isl29018.yaml | 13 +++++++++++++ 1 file changed, 13 insertions(+)Documentation/devicetree/bindings/iio/light/isl29018.yaml +0
-0@@ -34,6 +34,19 @@ properties: vcc-supply: description: Regulator that provides power to the sensorQuoted 6 lines
Documentation/devicetree/bindings/iio/light/isl29018.yaml properties:
Quoted 6 lines
Documentation/devicetree/bindings/iio/light/isl29018.yaml +6
-0+ 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.Is it useful to support decimal points on these values? The userspace interface does and you mention the 'right' answer might be only a few which means precision at that range will be terrible - less of an issue if 100s!
Thanks
Quoted 8 lines
Quoted 2 lines
Jonathan
Hard to say, my old HP TouchPad needs 100 as a value here (taken from legacy 2.6.35 kernel and binaries). So we probably don't need precision, but I have no other references to substantiate.
Thanks
Herman
Scratch that. Did some more research. Proof is in the legacy webOS binaries:
What legacy webOS actually does here:
1. Per-device calibration via factory token reads an ALSCal token from system storage containing JSON with calibration points at lux_0, lux_50, lux_100, lux_400 (measured ADC counts at known reference illuminance levels).
2. Computes a floating-point ratio AlsToLux_Ratio_WhiteLED = average of expected_lux / measured_count across the JSON calibration points. This is a real number, not an integer.
3. Adjusts for light source spectrum at runtime detects illuminant type from ALS:IR ratio, then applies a fractional spectrum correction:
Fluorescent above 100 counts: ratio x 0.4652
Incandescent above 700 counts: ratio x 0.4
Incandescent 50-100 counts: ratio x 0.9
Fluorescent < 50 counts: ratio x (-0.000724·N + 0.7463)
4. Final lux = ALSCount / spectrum_corrected_ratio - a true floating-point division.
Implications
- The "right" cover-comp value is per-device factory-measured, not per-board. Different units off the same production line have different optical transmission due to coating tolerance.
- The values are fractional by nature. Examples from the legacy code: 0.4652, 0.7463, 0.8333. None are integers.
The ISL29023 datasheet itself says nothing about cover compensation - it's strictly board-level optical correction. So there's no "right" answer from the chip side; it's whatever the board's cover glass + coating attenuates.
ALSCal values found on the particular device:
{"lux_50":{"c":31}, "lux_100":{"c":58}, "lux_400":{"c":164}}
This is device specific TouchPad's factory calibration:
- At 50 lux, ADC reads 31 counts lux/count = 1.613
- At 100 lux, ADC reads 58 counts lux/count = 1.724
- At 400 lux, ADC reads 164 counts lux/count = 2.439
Note the ratio isn't constant - the response is mildly non-linear, but per the legacy code the driver computes the average ratio as the calibration:
ratio = (1.6129 + 1.7241 + 2.4390) / 3 = 1.9253 lux/count
Independent verification:
- At calibscale=34: lux = 1295
- Implied raw count: 1295 / (34 x 0.015258) = 2496 counts
- Applying legacy formula: 2496 / 1.9253 = 1296.4 = 1295
The factory-calibrated value for this specific TouchPad is 34.04 (not 100). Per-point calibscale values from the ALSCal JSON:
Cal point‚ lux/count ratio‚ Equivalent calibscale
lux_50 ‚ 1.6129 ‚ 40.64 lux_100 ‚ 1.7241 ‚ 38.01 lux_400 ‚ 2.4390 ‚ 26.87 average ‚ 1.9253 ‚ 34.04
What this means concretely
1. Decimal precision is necessary, not nice-to-have. Real per-device factory values span 26.9 - 40.6 across the chip's response curve. A single scalar approximation costs precision; restricting to integer compounds it.
2. Updated v5 plan: switch to two-cell <int micro> for fractional values, and change the tenderloin DTS default from <100> to <34 040000> (or close to that).
Thoughts on this?
Thanks,
Herman
Quoted 29 lines
Quoted 29 lines
Quoted 29 lines
Documentation/devicetree/bindings/iio/light/isl29018.yaml properties: · line 44
Quoted 6 lines
The value seeds
Documentation/devicetree/bindings/iio/light/isl29018.yaml +2
-0+ in_illuminance0_calibscale, so it can still be retuned at + runtime through sysfs.Delete this, driver implementation stuff isn't relevant to the devicetree binding.
With that gone,
Acked-by Conor Dooley <[email protected]>pw-bot: changes-requested
Cheers,
Conor.Quoted 11 lines
Documentation/devicetree/bindings/iio/light/isl29018.yaml +5
-1+ $ref: /schemas/types.yaml#/definitions/uint32 + minimum: 1 + maximum: 65535 + default: 1 + required: - compatible - reg --2.43.0
Re: [PATCH v3 2/3] dt-bindings: iio: light: isl29018: add isil,cover-comp-gain
Quoted 133 lines · On Fri, Jun 05, 2026 at 09:28:03PM +0200, [email protected] wrote:
Quoted 43 lines · On 2026-06-05 15:18, [email protected] wrote:
Quoted 38 lines · Jonathan Cameron
Quoted 22 lines · On Thu, 4 Jun 2026 18:01:08 +0100 Conor Dooley <[email protected]> wrote:
Quoted 21 lines · 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 <[email protected]>--- .../devicetree/bindings/iio/light/isl29018.yaml | 13 +++++++++++++ 1 file changed, 13 insertions(+)Documentation/devicetree/bindings/iio/light/isl29018.yaml +0
-0@@ -34,6 +34,19 @@ properties: vcc-supply: description: Regulator that provides power to the sensorQuoted 6 lines
Documentation/devicetree/bindings/iio/light/isl29018.yaml properties:
Quoted 6 lines
Documentation/devicetree/bindings/iio/light/isl29018.yaml +6
-0+ 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.Is it useful to support decimal points on these values? The userspace interface
does and you mention the 'right' answer might be only a few which means precision
at that range will be terrible - less of an issue if 100s!Quoted 4 lines
Thanks
Jonathan
Quoted 8 lines
Hard to say, my old HP TouchPad needs 100 as a value here (taken from legacy 2.6.35
kernel and binaries). So we probably don't need precision, but I have no other
references to substantiate.Thanks
HermanScratch that. Did some more research. Proof is in the legacy webOS binaries:
What legacy webOS actually does here:
1. Per-device calibration via factory token reads an ALSCal token from system storage
containing JSON with calibration points atlux_0,lux_50,lux_100,lux_400(measured
ADC counts at known reference illuminance levels).2. Computes a floating-point ratio
AlsToLux_Ratio_WhiteLED= average ofexpected_lux/measured_countacross the JSON calibration points. This is a real number, not an integer.3. Adjusts for light source spectrum at runtime detects illuminant type from ALS:IR
ratio, then applies a fractional spectrum correction:
Fluorescent above 100 counts: ratio x 0.4652
Incandescent above 700 counts: ratio x 0.4
Incandescent 50-100 counts: ratio x 0.9
Fluorescent < 50 counts: ratio x (-0.000724·N + 0.7463)4. Final lux = ALSCount /
spectrum_corrected_ratio- a true floating-point division.Implications
- The "right" cover-comp value is per-device factory-measured, not per-board. Different
units off the same production line have different optical transmission due to coating
tolerance.
- The values are fractional by nature. Examples from the legacy code:
0.4652, 0.7463,
0.8333. None are integers.The ISL29023 datasheet itself says nothing about cover compensation - it's strictly
board-level optical correction. So there's no "right" answer from the chip side;
it's whatever the board's cover glass + coating attenuates.ALSCal values found on the particular device:
{"lux_50":{"c":31}, "lux_100":{"c":58}, "lux_400":{"c":164}}
This is device specific TouchPad's factory calibration:
- At 50 lux, ADC reads 31 counts lux/count = 1.613
- At 100 lux, ADC reads 58 counts lux/count = 1.724
- At 400 lux, ADC reads 164 counts lux/count = 2.439Note the ratio isn't constant - the response is mildly non-linear, but per the legacy
code the driver computes the average ratio as the calibration:ratio = (1.6129 + 1.7241 + 2.4390) / 3 = 1.9253 lux/count
Independent verification:
- At calibscale=34: lux = 1295
- Implied raw count: 1295 / (34 x 0.015258) = 2496 counts
- Applying legacy formula: 2496 / 1.9253 = 1296.4 = 1295The factory-calibrated value for this specific TouchPad is 34.04 (not 100). Per-point
calibscale values from the ALSCal JSON:Cal point‚ lux/count ratio‚ Equivalent calibscale
lux_50 ‚ 1.6129 ‚ 40.64 lux_100 ‚ 1.7241 ‚ 38.01 lux_400 ‚ 2.4390 ‚ 26.87 average ‚ 1.9253 ‚ 34.04What this means concretely
1. Decimal precision is necessary, not nice-to-have. Real per-device factory values
span 26.9 - 40.6 across the chip's response curve. A single scalar approximation costs
precision; restricting to integer compounds it.
Quoted 5 lines
2. Updated v5 plan: switch to two-cell <int micro> for fractional values, and change
the tenderloin DTS default from <100> to <34 040000> (or close to that).Thoughts on this?
Instead of going 2-cell, it might be worth moving the property to be based in some fractional unit to begin with. I just don't know what to call that, milligain? I dunno.
Jonathan's call here I think
Jonathan Cameron
maintainerRe: [PATCH v3 2/3] dt-bindings: iio: light: isl29018: add isil,cover-comp-gain
Quoted 138 lines · On Mon, 8 Jun 2026 18:49:13 +0100 Conor Dooley <[email protected]> wrote:
Quoted 137 lines · On Fri, Jun 05, 2026 at 09:28:03PM +0200, [email protected] wrote:
Quoted 43 lines · On 2026-06-05 15:18, [email protected] wrote:
Quoted 38 lines · Jonathan Cameron
Quoted 22 lines · On Thu, 4 Jun 2026 18:01:08 +0100 Conor Dooley <[email protected]> wrote:
Quoted 21 lines · 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 <[email protected]>--- .../devicetree/bindings/iio/light/isl29018.yaml | 13 +++++++++++++ 1 file changed, 13 insertions(+)Documentation/devicetree/bindings/iio/light/isl29018.yaml +0
-0@@ -34,6 +34,19 @@ properties: vcc-supply: description: Regulator that provides power to the sensorQuoted 6 lines
Documentation/devicetree/bindings/iio/light/isl29018.yaml properties:
Quoted 6 lines
Documentation/devicetree/bindings/iio/light/isl29018.yaml +6
-0+ 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.Is it useful to support decimal points on these values? The userspace interface
does and you mention the 'right' answer might be only a few which means precision
at that range will be terrible - less of an issue if 100s!Quoted 4 lines
Thanks
Jonathan
Quoted 8 lines
Hard to say, my old HP TouchPad needs 100 as a value here (taken from legacy 2.6.35
kernel and binaries). So we probably don't need precision, but I have no other
references to substantiate.Thanks
HermanScratch that. Did some more research. Proof is in the legacy webOS binaries:
What legacy webOS actually does here:
1. Per-device calibration via factory token reads an ALSCal token from system storage
containing JSON with calibration points atlux_0,lux_50,lux_100,lux_400(measured
ADC counts at known reference illuminance levels).2. Computes a floating-point ratio
AlsToLux_Ratio_WhiteLED= average ofexpected_lux/measured_countacross the JSON calibration points. This is a real number, not an integer.3. Adjusts for light source spectrum at runtime detects illuminant type from ALS:IR
ratio, then applies a fractional spectrum correction:
Fluorescent above 100 counts: ratio x 0.4652
Incandescent above 700 counts: ratio x 0.4
Incandescent 50-100 counts: ratio x 0.9
Fluorescent < 50 counts: ratio x (-0.000724·N + 0.7463)4. Final lux = ALSCount /
spectrum_corrected_ratio- a true floating-point division.Implications
- The "right" cover-comp value is per-device factory-measured, not per-board. Different
units off the same production line have different optical transmission due to coating
tolerance.
- The values are fractional by nature. Examples from the legacy code:
0.4652, 0.7463,
0.8333. None are integers.The ISL29023 datasheet itself says nothing about cover compensation - it's strictly
board-level optical correction. So there's no "right" answer from the chip side;
it's whatever the board's cover glass + coating attenuates.ALSCal values found on the particular device:
{"lux_50":{"c":31}, "lux_100":{"c":58}, "lux_400":{"c":164}}
This is device specific TouchPad's factory calibration:
- At 50 lux, ADC reads 31 counts lux/count = 1.613
- At 100 lux, ADC reads 58 counts lux/count = 1.724
- At 400 lux, ADC reads 164 counts lux/count = 2.439Note the ratio isn't constant - the response is mildly non-linear, but per the legacy
code the driver computes the average ratio as the calibration:ratio = (1.6129 + 1.7241 + 2.4390) / 3 = 1.9253 lux/count
Independent verification:
- At calibscale=34: lux = 1295
- Implied raw count: 1295 / (34 x 0.015258) = 2496 counts
- Applying legacy formula: 2496 / 1.9253 = 1296.4 = 1295The factory-calibrated value for this specific TouchPad is 34.04 (not 100). Per-point
calibscale values from the ALSCal JSON:Cal point‚ lux/count ratio‚ Equivalent calibscale
lux_50 ‚ 1.6129 ‚ 40.64 lux_100 ‚ 1.7241 ‚ 38.01 lux_400 ‚ 2.4390 ‚ 26.87 average ‚ 1.9253 ‚ 34.04What this means concretely
1. Decimal precision is necessary, not nice-to-have. Real per-device factory values
span 26.9 - 40.6 across the chip's response curve. A single scalar approximation costs
precision; restricting to integer compounds it.2. Updated v5 plan: switch to two-cell <int micro> for fractional values, and change
the tenderloin DTS default from <100> to <34 040000> (or close to that).
Quoted 8 lines
Quoted 1 line
Thoughts on this?
Instead of going 2-cell, it might be worth moving the property to be based in some fractional unit to begin with. I just don't know what to call that, milligain? I dunno.
Jonathan's call here I think
If this is varying that a lot per device, then, unless you plan to have a bootloader inject it into DT, are we better off making this a userspace problem? There is a hybrid we've done before, which is to make it a firmware loading problem but that requires some way to generate a suitable file to load and typically a calibration sequence that goes with it.
If it is 'fairly constant' for a given device then fine to be in DT
- it will just be a case of picking the average of a bunch of measured devices and assuming it is good enough.
If it does end up in dt, then a fractional type like Conor suggests is fine.
Jonathan
Re: [PATCH v3 2/3] dt-bindings: iio: light: isl29018: add isil,cover-comp-gain
Quoted 157 lines · Jonathan Cameron
Quoted 138 lines · On Mon, 8 Jun 2026 18:49:13 +0100 Conor Dooley <[email protected]> wrote:
Quoted 137 lines · On Fri, Jun 05, 2026 at 09:28:03PM +0200, [email protected] wrote:
Quoted 43 lines · On 2026-06-05 15:18, [email protected] wrote:
Quoted 38 lines · Jonathan Cameron
Quoted 22 lines · On Thu, 4 Jun 2026 18:01:08 +0100 Conor Dooley <[email protected]> wrote:
Quoted 21 lines · 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 <[email protected]>--- .../devicetree/bindings/iio/light/isl29018.yaml | 13 +++++++++++++ 1 file changed, 13 insertions(+)Documentation/devicetree/bindings/iio/light/isl29018.yaml +0
-0@@ -34,6 +34,19 @@ properties: vcc-supply: description: Regulator that provides power to the sensorQuoted 6 lines
Documentation/devicetree/bindings/iio/light/isl29018.yaml properties: · line 42
Quoted 6 lines
Documentation/devicetree/bindings/iio/light/isl29018.yaml +6
-0+ 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.Is it useful to support decimal points on these values? The userspace interface
does and you mention the 'right' answer might be only a few which means precision
at that range will be terrible - less of an issue if 100s!Quoted 4 lines
Thanks
Jonathan
Quoted 8 lines
Hard to say, my old HP TouchPad needs 100 as a value here (taken from legacy 2.6.35
kernel and binaries). So we probably don't need precision, but I have no other
references to substantiate.Thanks
HermanScratch that. Did some more research. Proof is in the legacy webOS binaries:
What legacy webOS actually does here:
1. Per-device calibration via factory token reads an ALSCal token from system storage
containing JSON with calibration points atlux_0,lux_50,lux_100,lux_400(measured
ADC counts at known reference illuminance levels).2. Computes a floating-point ratio
AlsToLux_Ratio_WhiteLED= average ofexpected_lux/measured_countacross the JSON calibration points. This is a real number, not an integer.3. Adjusts for light source spectrum at runtime detects illuminant type from ALS:IR
ratio, then applies a fractional spectrum correction:
Fluorescent above 100 counts: ratio x 0.4652
Incandescent above 700 counts: ratio x 0.4
Incandescent 50-100 counts: ratio x 0.9
Fluorescent < 50 counts: ratio x (-0.000724·N + 0.7463)4. Final lux = ALSCount /
spectrum_corrected_ratio- a true floating-point division.Implications
- The "right" cover-comp value is per-device factory-measured, not per-board. Different
units off the same production line have different optical transmission due to coating
tolerance.
- The values are fractional by nature. Examples from the legacy code:
0.4652, 0.7463,
0.8333. None are integers.The ISL29023 datasheet itself says nothing about cover compensation - it's strictly
board-level optical correction. So there's no "right" answer from the chip side;
it's whatever the board's cover glass + coating attenuates.ALSCal values found on the particular device:
{"lux_50":{"c":31}, "lux_100":{"c":58}, "lux_400":{"c":164}}
This is device specific TouchPad's factory calibration:
- At 50 lux, ADC reads 31 counts lux/count = 1.613
- At 100 lux, ADC reads 58 counts lux/count = 1.724
- At 400 lux, ADC reads 164 counts lux/count = 2.439Note the ratio isn't constant - the response is mildly non-linear, but per the legacy
code the driver computes the average ratio as the calibration:ratio = (1.6129 + 1.7241 + 2.4390) / 3 = 1.9253 lux/count
Independent verification:
- At calibscale=34: lux = 1295
- Implied raw count: 1295 / (34 x 0.015258) = 2496 counts
- Applying legacy formula: 2496 / 1.9253 = 1296.4 = 1295The factory-calibrated value for this specific TouchPad is 34.04 (not 100). Per-point
calibscale values from the ALSCal JSON:Cal point‚ lux/count ratio‚ Equivalent calibscale
lux_50 ‚ 1.6129 ‚ 40.64 lux_100 ‚ 1.7241 ‚ 38.01 lux_400 ‚ 2.4390 ‚ 26.87 average ‚ 1.9253 ‚ 34.04What this means concretely
1. Decimal precision is necessary, not nice-to-have. Real per-device factory values
span 26.9 - 40.6 across the chip's response curve. A single scalar approximation costs
precision; restricting to integer compounds it.2. Updated v5 plan: switch to two-cell <int micro> for fractional values, and change
the tenderloin DTS default from <100> to <34 040000> (or close to that).Quoted 8 lines
Quoted 1 line
Thoughts on this?
Instead of going 2-cell, it might be worth moving the property to be based in some fractional unit to begin with. I just don't know what to call that, milligain? I dunno.
Jonathan's call here I think
If this is varying that a lot per device, then, unless you plan to have a bootloader inject it into DT, are we better off making this a userspace problem? There is a hybrid we've done before, which is to make it a firmware loading problem but that requires some way to generate a suitable file to load and typically a calibration sequence that goes with it.
Quoted 8 lines
If it is 'fairly constant' for a given device then fine to be in DT
- it will just be a case of picking the average of a bunch of measured devices and assuming it is good enough.If it does end up in dt, then a fractional type like Conor suggests is fine.
Jonathan
Hi Jonathan,
I have about 8-10 devices in slightly different configurations using this I guess, so I can quickly run the numbers and see how the values look like, so we have more data to go by.
Thanks,
Herman