[PATCH] iio: hid-sensor: als: scale each channel individually

[PATCH] iio: hid-sensor: als: scale each channel individually

[Daniel Schaefer via B4 Relay]

From: Daniel Schaefer <dhs@frame.work>

Some sensors have multiple channels (not just brightness but color ALS
sensors) with different scaling factors.

Tested on Asus ProArt PX13 laptop by Sven.

Signed-off-by: Daniel Schaefer <dhs@frame.work>
Link: https://gitlab.freedesktop.org/hadess/iio-sensor-proxy/-/merge_requests/413
Cc: Sven Vainküla <sven@xn--vainkla-r2a.ee>
Cc: Jiri Kosina <jikos@kernel.org>
Cc: Jonathan Cameron <jic23@kernel.org>
Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Cc: David Lechner <dlechner@baylibre.com>
Cc: Nuno Sá <nuno.sa@analog.com>
Cc: Andy Shevchenko <andy@kernel.org>
Cc: linux-input@vger.kernel.org
Cc: linux-iio@vger.kernel.org
Cc: linux@frame.work
---
 drivers/iio/light/hid-sensor-als.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/drivers/iio/light/hid-sensor-als.c b/drivers/iio/light/hid-sensor-als.c
index 384572844162..232cb58e0a9f 100644
--- a/drivers/iio/light/hid-sensor-als.c
+++ b/drivers/iio/light/hid-sensor-als.c
@@ -33,9 +33,9 @@ struct als_state {
 		u32 illum[CHANNEL_SCAN_INDEX_MAX];
 		aligned_s64 timestamp;
 	} scan;
-	int scale_pre_decml;
-	int scale_post_decml;
-	int scale_precision;
+	int scale_pre_decml[CHANNEL_SCAN_INDEX_MAX];
+	int scale_post_decml[CHANNEL_SCAN_INDEX_MAX];
+	int scale_precision[CHANNEL_SCAN_INDEX_MAX];
 	int value_offset;
 	int num_channels;
 	s64 timestamp;
@@ -186,9 +186,11 @@ static int als_read_raw(struct iio_dev *indio_dev,
 		ret_type = IIO_VAL_INT;
 		break;
 	case IIO_CHAN_INFO_SCALE:
-		*val = als_state->scale_pre_decml;
-		*val2 = als_state->scale_post_decml;
-		ret_type = als_state->scale_precision;
+		if (chan->scan_index >= CHANNEL_SCAN_INDEX_MAX)
+			return -EINVAL;
+		*val = als_state->scale_pre_decml[chan->scan_index];
+		*val2 = als_state->scale_post_decml[chan->scan_index];
+		ret_type = als_state->scale_precision[chan->scan_index];
 		break;
 	case IIO_CHAN_INFO_OFFSET:
 		*val = als_state->value_offset;
@@ -338,6 +340,10 @@ static int als_parse_report(struct platform_device *pdev,
 		als_adjust_channel_bit_mask(channels, index, st->als[i].size);
 		++index;
 
+		st->scale_precision[i] = hid_sensor_format_scale(usage_id,
+					&st->als[i], &st->scale_pre_decml[i],
+					&st->scale_post_decml[i]);
+
 		dev_dbg(&pdev->dev, "als %x:%x\n", st->als[i].index,
 			st->als[i].report_id);
 	}
@@ -347,10 +353,6 @@ static int als_parse_report(struct platform_device *pdev,
 	if (index)
 		ret = 0;
 
-	st->scale_precision = hid_sensor_format_scale(usage_id,
-				&st->als[CHANNEL_SCAN_INDEX_INTENSITY],
-				&st->scale_pre_decml, &st->scale_post_decml);
-
 	return ret;
 }
 

---
base-commit: 194a48576843858afcd9c00ed4f105ec2b398e6c
change-id: 20260723-chromaticity-88fec417f00c

Best regards,
-- 
Daniel Schaefer <dhs@frame.work>

Re: [PATCH] iio: hid-sensor: als: scale each channel individually

[sashiko-bot]

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

New issues:
- [Medium] The patch fails to expose the individual scale for the CHROMATICITY_Y channel because it still uses `info_mask_shared_by_type`.

Pre-existing issues:
- [High] The driver's IIO buffer layout uses a fixed structure, but `available_scan_masks` is dynamically populated, causing misaligned buffer reads and data corruption.
--

--- Patch [1]: [PATCH] iio: hid-sensor: als: scale each channel individually ---
commit 6d49cfd4cf380845a584116e06dcf23fc8c25ad6
Author: Daniel Schaefer <dhs@frame.work>

iio: hid-sensor: als: scale each channel individually

This commit updates the hid-sensor-als driver to support individual scaling
factors for each channel, instead of using a single global scale. It stores
the scale attributes in arrays indexed by channel.

Link: https://gitlab.freedesktop.org/hadess/iio-sensor-proxy/-/merge_requests/413

> diff --git a/drivers/iio/light/hid-sensor-als.c b/drivers/iio/light/hid-sensor-als.c
> index 384572844162c..232cb58e0a9ff 100644
> --- a/drivers/iio/light/hid-sensor-als.c
> +++ b/drivers/iio/light/hid-sensor-als.c
> @@ -33,9 +33,9 @@ struct als_state {
>  		u32 illum[CHANNEL_SCAN_INDEX_MAX];
>  		aligned_s64 timestamp;
>  	} scan;

[Severity: High]
This is a pre-existing issue, but does this fixed struct buffer layout
cause data corruption?

The driver captures samples in als_capture_sample() into a static sparse
array (als_state->scan.illum) based on fixed channel indices. However,
indio_dev->available_scan_masks is dynamically constructed based on the
attributes present on the sensor.

The IIO core calculates scan_bytes and channel offsets assuming the buffer is
tightly packed.

When iio_push_to_buffers_with_ts() is called in als_proc_event(), it writes
the timestamp at the end of the tightly packed length, which overwrites valid
channel data in the sparse array.

The IIO demuxer then reads from incorrect offsets, scrambling the data sent to
userspace.

Should the driver pack the samples sequentially instead of using fixed index
offsets?

> -	int scale_pre_decml;
> -	int scale_post_decml;
> -	int scale_precision;
> +	int scale_pre_decml[CHANNEL_SCAN_INDEX_MAX];
> +	int scale_post_decml[CHANNEL_SCAN_INDEX_MAX];
> +	int scale_precision[CHANNEL_SCAN_INDEX_MAX];
>  	int value_offset;
>  	int num_channels;
>  	s64 timestamp;

[ ... ]

> @@ -186,9 +186,11 @@ static int als_read_raw(struct iio_dev *indio_dev,
>  		ret_type = IIO_VAL_INT;
>  		break;
>  	case IIO_CHAN_INFO_SCALE:
> -		*val = als_state->scale_pre_decml;
> -		*val2 = als_state->scale_post_decml;
> -		ret_type = als_state->scale_precision;
> +		if (chan->scan_index >= CHANNEL_SCAN_INDEX_MAX)
> +			return -EINVAL;
> +		*val = als_state->scale_pre_decml[chan->scan_index];
> +		*val2 = als_state->scale_post_decml[chan->scan_index];
> +		ret_type = als_state->scale_precision[chan->scan_index];
>  		break;

[Severity: Medium]
Since the patch introduces per-channel scaling factors, can userspace actually
read the scale for the CHROMATICITY_Y channel?

The als_channels array defines both CHANNEL_SCAN_INDEX_CHROMATICITY_X and Y
with .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE).

Because of this, the IIO core will create a single in_chromaticity_scale sysfs
attribute for both.

When read, IIO queries the first registered channel of that type (X), leaving
the Y scale factor completely inaccessible.

Does the info_mask_shared_by_type for these channels need to be changed to
info_mask_separate so userspace can read the newly added scale for
CHROMATICITY_Y?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260723-chromaticity-v1-1-5ef5746ace2a@frame.work?part=1