linux-kernel - Re: [PATCH v5 2/3] iio: light: add support for veml6046x00 RGBIR color sensor

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20250531182031.453ca161@jic23-huawei>
Date: Sat, 31 May 2025 18:20:31 +0100
From: Jonathan Cameron <jic23@...nel.org>
To: Andreas Klinger <ak@...klinger.de>
Cc: robh@...nel.org, krzk+dt@...nel.org, conor+dt@...nel.org,
 lars@...afoo.de, javier.carrasco.cruz@...il.com, mazziesaccount@...il.com,
 andriy.shevchenko@...ux.intel.com, arthur.becker@...tec.com,
 perdaniel.olsson@...s.com, mgonellabolduc@...onoff.com,
 muditsharma.info@...il.com, clamor95@...il.com, emil.gedenryd@...s.com,
 devicetree@...r.kernel.org, linux-iio@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH v5 2/3] iio: light: add support for veml6046x00 RGBIR
 color sensor

On Mon, 26 May 2025 10:50:40 +0200
Andreas Klinger <ak@...klinger.de> wrote:

> Add Vishay VEML6046X00 high accuracy RGBIR color sensor.
> 
> This sensor provides three colour (red, green and blue) as well as one
> infrared (IR) channel through I2C.
> 
> Support direct and buffered mode.
> 
> An optional interrupt for signaling green colour threshold underflow or
> overflow is not supported so far.
> 
> Signed-off-by: Andreas Klinger <ak@...klinger.de>

Hi Andreas,

I missed a channel type issue. See below.

> diff --git a/drivers/iio/light/veml6046x00.c b/drivers/iio/light/veml6046x00.c
> new file mode 100644
> index 000000000000..9972eeb57fd2
> --- /dev/null
> +++ b/drivers/iio/light/veml6046x00.c
> @@ -0,0 +1,1007 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * VEML6046X00 High Accuracy RGBIR Color Sensor
> + *
> + * Copyright (c) 2025 Andreas Klinger <ak@...klinger.de>
> + */

> +static const struct iio_chan_spec veml6046x00_channels[] = {
> +	{
> +		.type = IIO_LIGHT,

Sorry - I've been half asleep on earlier versions or maybe we discussed this
before and I've forgotten.  IIO_LIGHT is for illuminance with units of lux.

How does that apply to a colour channel given it's a measure of a specific
sensitivity curve for the human eye and such a thing only applies to 'whiteish'
light.

Normally we cover colour channels as IIO_INTENSITY which doesn't have
strong rules for scaling - so lets us get away with the many weird and wonderful
ideals different sensor manufacturers have of what RED / GREEN / BLUE
mean.  (There is an oddity for historical reasons IIRC of an IR light channel
but don't use that for new code).

So basically the request is to use .type = IIO_INTENSITY for these

> +		.address = VEML6046X00_REG_R,
> +		.modified = 1,
> +		.channel2 = IIO_MOD_LIGHT_RED,
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
> +		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_INT_TIME) |
> +					   BIT(IIO_CHAN_INFO_SCALE),
> +		.info_mask_shared_by_all_available = BIT(IIO_CHAN_INFO_INT_TIME) |
> +						     BIT(IIO_CHAN_INFO_SCALE),
> +		.scan_index = VEML6046X00_SCAN_R,
> +		.scan_type = {
> +			.sign = 'u',
> +			.realbits = 16,
> +			.storagebits = 16,
> +			.endianness = IIO_LE,
> +		},
> +	},
> +	{
> +		.type = IIO_LIGHT,
> +		.address = VEML6046X00_REG_G,
> +		.modified = 1,
> +		.channel2 = IIO_MOD_LIGHT_GREEN,
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
> +		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_INT_TIME) |
> +					   BIT(IIO_CHAN_INFO_SCALE),
> +		.info_mask_shared_by_all_available = BIT(IIO_CHAN_INFO_INT_TIME) |
> +						     BIT(IIO_CHAN_INFO_SCALE),
> +		.scan_index = VEML6046X00_SCAN_G,
> +		.scan_type = {
> +			.sign = 'u',
> +			.realbits = 16,
> +			.storagebits = 16,
> +			.endianness = IIO_LE,
> +		},
> +	},
> +	{
> +		.type = IIO_LIGHT,
> +		.address = VEML6046X00_REG_B,
> +		.modified = 1,
> +		.channel2 = IIO_MOD_LIGHT_BLUE,
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
> +		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_INT_TIME) |
> +					   BIT(IIO_CHAN_INFO_SCALE),
> +		.info_mask_shared_by_all_available = BIT(IIO_CHAN_INFO_INT_TIME) |
> +						     BIT(IIO_CHAN_INFO_SCALE),
> +		.scan_index = VEML6046X00_SCAN_B,
> +		.scan_type = {
> +			.sign = 'u',
> +			.realbits = 16,
> +			.storagebits = 16,
> +			.endianness = IIO_LE,
> +		},
> +	},
> +	{
> +		.type = IIO_LIGHT,
> +		.address = VEML6046X00_REG_IR,
> +		.modified = 1,
> +		.channel2 = IIO_MOD_LIGHT_IR,
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
> +		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_INT_TIME) |
> +					   BIT(IIO_CHAN_INFO_SCALE),
> +		.info_mask_shared_by_all_available = BIT(IIO_CHAN_INFO_INT_TIME) |
> +						     BIT(IIO_CHAN_INFO_SCALE),
> +		.scan_index = VEML6046X00_SCAN_IR,
> +		.scan_type = {
> +			.sign = 'u',
> +			.realbits = 16,
> +			.storagebits = 16,
> +			.endianness = IIO_LE,
> +		},
> +	},
> +	IIO_CHAN_SOFT_TIMESTAMP(VEML6046X00_SCAN_TIMESTAMP),
> +};