[PATCH v2 0/3] iio: light: add support for AMS AS7331

[PATCH v2 0/3] iio: light: add support for AMS AS7331

[Javier Carrasco]

The AMS AS7331 UV light sensor measures three ultraviolet bands (UVA,
UVB and UVC, also known as deep UV or DUV) as well as temperature.

This device is practically identical to the AMS AS73211 XYZ True Color
sensor that is already supported by the iio subsystem, except for the
photodiodes used to aquire the desired light wavelengths.

In order to reuse code and reduce maintenance load, this series extends
the AS73211 driver to support the AS7331 as well.

Note that the UVA and UVB light modifiers have not been merged into the
mainline kernel yet, but they are already available in Greg's char-misc
git tree which can be found at
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git
in the char-misc-next branch.

The original device AS73211 supported by the driver could only be tested
briefly due to the lack of hardware. Instead, the i2c-stub module has
been used to make sure that the driver registers the iio device properly
and the attributes exported to sysfs are correct. Some basic register
assignments reported the expected intensity scales and in principle
nothing else should have been affected by the modifications in the code.

Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
---
Changes in v2:
- as73211.c: Use IIO_VAL_FRACTIONAL to retrieve scales of AS73211.
- as73211.c: simplify device-specific data retrieval in probe function.
- as73211.c: minor coding-style fix (shorter line).
- Link to v1: https://lore.kernel.org/r/20231220-as7331-v1-0-745b73c27703@gmail.com

---
Javier Carrasco (3):
      iio: light: as73211: use IIO_VAL_FRACTIONAL for intensity scales
      dt-bindings: iio: light: as73211: add support for as7331
      iio: light: as73211: add support for as7331

 .../devicetree/bindings/iio/light/ams,as73211.yaml |   7 +-
 drivers/iio/light/Kconfig                          |   5 +-
 drivers/iio/light/as73211.c                        | 142 +++++++++++++++++----
 3 files changed, 123 insertions(+), 31 deletions(-)
---
base-commit: e9215fcca2561b208c78359110ee4009b454f761
change-id: 20231220-as7331-88a25ceeb66d

Best regards,
-- 
Javier Carrasco <javier.carrasco.cruz@gmail.com>

[PATCH v2 1/3] iio: light: as73211: use IIO_VAL_FRACTIONAL for intensity scales

[Javier Carrasco]

The scale values associated to the light channels are calculated as a
division that can be better expressed as an IIO_VAL_FRACTIONAL type
instead of the current IIO_VAL_INT.

Note that the constant values used for the calculation were scaled up to
work with integers, turning the nW/cm^2 units from the datasheet into
nW/m^2, which would not be necessary with the IIO_VAL_FRACTIONAL type.
But to avoid issues from current users of the driver, the units must be
kept.

Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
---
 drivers/iio/light/as73211.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/iio/light/as73211.c b/drivers/iio/light/as73211.c
index ec97a3a46839..b4c6f389a292 100644
--- a/drivers/iio/light/as73211.c
+++ b/drivers/iio/light/as73211.c
@@ -356,25 +356,24 @@ static int as73211_read_raw(struct iio_dev *indio_dev, struct iio_chan_spec cons
 			return IIO_VAL_INT_PLUS_MICRO;
 
 		case IIO_INTENSITY: {
-			unsigned int scale;
 
 			switch (chan->channel2) {
 			case IIO_MOD_X:
-				scale = AS73211_SCALE_X;
+				*val = AS73211_SCALE_X;
 				break;
 			case IIO_MOD_Y:
-				scale = AS73211_SCALE_Y;
+				*val = AS73211_SCALE_Y;
 				break;
 			case IIO_MOD_Z:
-				scale = AS73211_SCALE_Z;
+				*val = AS73211_SCALE_Z;
 				break;
 			default:
 				return -EINVAL;
 			}
-			scale /= as73211_gain(data);
-			scale /= as73211_integration_time_1024cyc(data);
-			*val = scale;
-			return IIO_VAL_INT;
+			*val2 = as73211_integration_time_1024cyc(data) *
+				as73211_gain(data);
+
+			return IIO_VAL_FRACTIONAL;
 
 		default:
 			return -EINVAL;

-- 
2.39.2

[PATCH v2 2/3] dt-bindings: iio: light: as73211: add support for as7331

[Javier Carrasco]

This device has the same properties and I2C addresses as the as73211.
The only difference between them is the photodiodes they use internally,
which in this case is irrelevant for the bindings.

Acked-by: Conor Dooley <conor.dooley@microchip.com>
Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
---
 Documentation/devicetree/bindings/iio/light/ams,as73211.yaml | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/iio/light/ams,as73211.yaml b/Documentation/devicetree/bindings/iio/light/ams,as73211.yaml
index 0e8cd02759b3..062a038aa0ff 100644
--- a/Documentation/devicetree/bindings/iio/light/ams,as73211.yaml
+++ b/Documentation/devicetree/bindings/iio/light/ams,as73211.yaml
@@ -4,19 +4,22 @@
 $id: http://devicetree.org/schemas/iio/light/ams,as73211.yaml#
 $schema: http://devicetree.org/meta-schemas/core.yaml#
 
-title: AMS AS73211 JENCOLOR(R) Digital XYZ Sensor
+title: AMS AS73211 JENCOLOR(R) Digital XYZ Sensor and AMS AS7331 UV Sensor
 
 maintainers:
   - Christian Eggers <ceggers@arri.de>
 
 description: |
-  XYZ True Color Sensor with I2C Interface
+  AMS AS73211 XYZ True Color Sensor with I2C Interface
   https://ams.com/documents/20143/36005/AS73211_DS000556_3-01.pdf/a65474c0-b302-c2fd-e30a-c98df87616df
+  AMS AS7331 UVA, UVB and UVC Sensor with I2C Interface
+  https://ams.com/documents/20143/9106314/AS7331_DS001047_4-00.pdf
 
 properties:
   compatible:
     enum:
       - ams,as73211
+      - ams,as7331
 
   reg:
     description:

-- 
2.39.2

[PATCH v2 3/3] iio: light: as73211: add support for as7331

[Javier Carrasco]

The AMS AS7331 is a UV light sensor with three channels: UVA, UVB and
UVC (also known as deep UV and referenced as DUV in the iio core).
Its internal structure and forming blocks are practically identical to
the ones the AS73211 contains: API, internal DAC, I2C interface and
registers, measurement modes, number of channels and pinout.

The only difference between them is the photodiodes used to acquire
light, which means that only some modifications are required to add
support for the AS7331 in the existing driver.

The temperature channel is identical for both devices and only the
channel modifiers of the IIO_INTENSITY channels need to account for the
device type.

The scale values have been obtained from the chapter "7.5 Transfer
Function" of the official datasheet[1] for the configuration chosen as
basis (Nclk = 1024 and GAIN = 1). Those values keep the units from the
datasheet (nW/cm^2), as opposed to the units used for the AS73211
(nW/m^2).

Add a new device-specific data structure to account for the device
differences: channel types and scale of LSB per channel.

[1] https://ams.com/documents/20143/9106314/AS7331_DS001047_4-00.pdf

Tested-by: Christian Eggers <ceggers@arri.de>
Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
---
 drivers/iio/light/Kconfig   |   5 +-
 drivers/iio/light/as73211.c | 141 ++++++++++++++++++++++++++++++++++++--------
 2 files changed, 118 insertions(+), 28 deletions(-)

diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig
index 143003232d1c..fd5a9879a582 100644
--- a/drivers/iio/light/Kconfig
+++ b/drivers/iio/light/Kconfig
@@ -87,13 +87,14 @@ config APDS9960
 	  module will be called apds9960
 
 config AS73211
-	tristate "AMS AS73211 XYZ color sensor"
+	tristate "AMS AS73211 XYZ color sensor and AMS AS7331 UV sensor"
 	depends on I2C
 	select IIO_BUFFER
 	select IIO_TRIGGERED_BUFFER
 	help
 	 If you say yes here you get support for the AMS AS73211
-	 JENCOLOR(R) Digital XYZ Sensor.
+	 JENCOLOR(R) Digital XYZ and the AMS AS7331 UVA, UVB and UVC
+	 ultraviolet sensors.
 
 	 For triggered measurements, you will need an additional trigger driver
 	 like IIO_HRTIMER_TRIGGER or IIO_SYSFS_TRIGGER.
diff --git a/drivers/iio/light/as73211.c b/drivers/iio/light/as73211.c
index b4c6f389a292..44daf816ae57 100644
--- a/drivers/iio/light/as73211.c
+++ b/drivers/iio/light/as73211.c
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0-only
 /*
- * Support for AMS AS73211 JENCOLOR(R) Digital XYZ Sensor
+ * Support for AMS AS73211 JENCOLOR(R) Digital XYZ Sensor and AMS AS7331
+ * UVA, UVB and UVC (DUV) Ultraviolet Sensor
  *
  * Author: Christian Eggers <ceggers@arri.de>
  *
@@ -9,7 +10,9 @@
  * Color light sensor with 16-bit channels for x, y, z and temperature);
  * 7-bit I2C slave address 0x74 .. 0x77.
  *
- * Datasheet: https://ams.com/documents/20143/36005/AS73211_DS000556_3-01.pdf
+ * Datasheets:
+ * AS73211: https://ams.com/documents/20143/36005/AS73211_DS000556_3-01.pdf
+ * AS7331: https://ams.com/documents/20143/9106314/AS7331_DS001047_4-00.pdf
  */
 
 #include <linux/bitfield.h>
@@ -84,6 +87,20 @@ static const int as73211_hardwaregain_avail[] = {
 	1, 2, 4, 8, 16, 32, 64, 128, 256, 512, 1024, 2048,
 };
 
+struct as73211_data;
+
+/**
+ * struct spec_dev_data - device-specific data
+ * @intensity_scale:  Function to retrieve intensity scale values.
+ * @channel:          Device channels.
+ * @num_channels:     Number of channels of the device.
+ */
+struct spec_dev_data {
+	int (*intensity_scale)(struct as73211_data *data, int chan, int *val, int *val2);
+	struct iio_chan_spec const *channel;
+	int num_channels;
+};
+
 /**
  * struct as73211_data - Instance data for one AS73211
  * @client: I2C client.
@@ -94,6 +111,7 @@ static const int as73211_hardwaregain_avail[] = {
  * @mutex:  Keeps cached registers in sync with the device.
  * @completion: Completion to wait for interrupt.
  * @int_time_avail: Available integration times (depend on sampling frequency).
+ * @spec_dev: device-specific configuration.
  */
 struct as73211_data {
 	struct i2c_client *client;
@@ -104,6 +122,7 @@ struct as73211_data {
 	struct mutex mutex;
 	struct completion completion;
 	int int_time_avail[AS73211_SAMPLE_TIME_NUM * 2];
+	const struct spec_dev_data *spec_dev;
 };
 
 #define AS73211_COLOR_CHANNEL(_color, _si, _addr) { \
@@ -138,6 +157,10 @@ struct as73211_data {
 #define AS73211_SCALE_Y 298384270  /* nW/m^2 */
 #define AS73211_SCALE_Z 160241927  /* nW/m^2 */
 
+#define AS7331_SCALE_UVA 340000  /* nW/cm^2 */
+#define AS7331_SCALE_UVB 378000  /* nW/cm^2 */
+#define AS7331_SCALE_UVC 166000  /* nW/cm^2 */
+
 /* Channel order MUST match devices result register order */
 #define AS73211_SCAN_INDEX_TEMP 0
 #define AS73211_SCAN_INDEX_X    1
@@ -176,6 +199,28 @@ static const struct iio_chan_spec as73211_channels[] = {
 	IIO_CHAN_SOFT_TIMESTAMP(AS73211_SCAN_INDEX_TS),
 };
 
+static const struct iio_chan_spec as7331_channels[] = {
+	{
+		.type = IIO_TEMP,
+		.info_mask_separate =
+			BIT(IIO_CHAN_INFO_RAW) |
+			BIT(IIO_CHAN_INFO_OFFSET) |
+			BIT(IIO_CHAN_INFO_SCALE),
+		.address = AS73211_OUT_TEMP,
+		.scan_index = AS73211_SCAN_INDEX_TEMP,
+		.scan_type = {
+			.sign = 'u',
+			.realbits = 16,
+			.storagebits = 16,
+			.endianness = IIO_LE,
+		}
+	},
+	AS73211_COLOR_CHANNEL(LIGHT_UVA, AS73211_SCAN_INDEX_X, AS73211_OUT_MRES1),
+	AS73211_COLOR_CHANNEL(LIGHT_UVB, AS73211_SCAN_INDEX_Y, AS73211_OUT_MRES2),
+	AS73211_COLOR_CHANNEL(LIGHT_DUV, AS73211_SCAN_INDEX_Z, AS73211_OUT_MRES3),
+	IIO_CHAN_SOFT_TIMESTAMP(AS73211_SCAN_INDEX_TS),
+};
+
 static unsigned int as73211_integration_time_1024cyc(struct as73211_data *data)
 {
 	/*
@@ -316,6 +361,48 @@ static int as73211_req_data(struct as73211_data *data)
 	return 0;
 }
 
+static int as73211_intensity_scale(struct as73211_data *data, int chan,
+				   int *val, int *val2)
+{
+	switch (chan) {
+	case IIO_MOD_X:
+		*val = AS73211_SCALE_X;
+		break;
+	case IIO_MOD_Y:
+		*val = AS73211_SCALE_Y;
+		break;
+	case IIO_MOD_Z:
+		*val = AS73211_SCALE_Z;
+		break;
+	default:
+		return -EINVAL;
+	}
+	*val2 = as73211_integration_time_1024cyc(data) * as73211_gain(data);
+
+	return IIO_VAL_FRACTIONAL;
+}
+
+static int as7331_intensity_scale(struct as73211_data *data, int chan,
+				  int *val, int *val2)
+{
+	switch (chan) {
+	case IIO_MOD_LIGHT_UVA:
+		*val = AS7331_SCALE_UVA;
+		break;
+	case IIO_MOD_LIGHT_UVB:
+		*val = AS7331_SCALE_UVB;
+		break;
+	case IIO_MOD_LIGHT_DUV:
+		*val = AS7331_SCALE_UVC;
+		break;
+	default:
+		return -EINVAL;
+	}
+	*val2 = as73211_integration_time_1024cyc(data) * as73211_gain(data);
+
+	return IIO_VAL_FRACTIONAL;
+}
+
 static int as73211_read_raw(struct iio_dev *indio_dev, struct iio_chan_spec const *chan,
 			     int *val, int *val2, long mask)
 {
@@ -355,29 +442,13 @@ static int as73211_read_raw(struct iio_dev *indio_dev, struct iio_chan_spec cons
 			*val2 = AS73211_SCALE_TEMP_MICRO;
 			return IIO_VAL_INT_PLUS_MICRO;
 
-		case IIO_INTENSITY: {
-
-			switch (chan->channel2) {
-			case IIO_MOD_X:
-				*val = AS73211_SCALE_X;
-				break;
-			case IIO_MOD_Y:
-				*val = AS73211_SCALE_Y;
-				break;
-			case IIO_MOD_Z:
-				*val = AS73211_SCALE_Z;
-				break;
-			default:
-				return -EINVAL;
-			}
-			*val2 = as73211_integration_time_1024cyc(data) *
-				as73211_gain(data);
-
-			return IIO_VAL_FRACTIONAL;
+		case IIO_INTENSITY:
+			return data->spec_dev->intensity_scale(data, chan->channel2,
+							       val, val2);
 
 		default:
 			return -EINVAL;
-		}}
+		}
 
 	case IIO_CHAN_INFO_SAMP_FREQ:
 		/* f_samp is configured in CREG3 in powers of 2 (x 1.024 MHz) */
@@ -675,13 +746,17 @@ static int as73211_probe(struct i2c_client *client)
 	i2c_set_clientdata(client, indio_dev);
 	data->client = client;
 
+	data->spec_dev = i2c_get_match_data(client);
+	if (!data->spec_dev)
+		return -EINVAL;
+
 	mutex_init(&data->mutex);
 	init_completion(&data->completion);
 
 	indio_dev->info = &as73211_info;
 	indio_dev->name = AS73211_DRV_NAME;
-	indio_dev->channels = as73211_channels;
-	indio_dev->num_channels = ARRAY_SIZE(as73211_channels);
+	indio_dev->channels = data->spec_dev->channel;
+	indio_dev->num_channels = data->spec_dev->num_channels;
 	indio_dev->modes = INDIO_DIRECT_MODE;
 
 	ret = i2c_smbus_read_byte_data(data->client, AS73211_REG_OSR);
@@ -771,14 +846,28 @@ static int as73211_resume(struct device *dev)
 static DEFINE_SIMPLE_DEV_PM_OPS(as73211_pm_ops, as73211_suspend,
 				as73211_resume);
 
+static const struct spec_dev_data as73211_spec = {
+	.intensity_scale = as73211_intensity_scale,
+	.channel = as73211_channels,
+	.num_channels = ARRAY_SIZE(as73211_channels),
+};
+
+static const struct spec_dev_data as7331_spec = {
+	.intensity_scale = as7331_intensity_scale,
+	.channel = as7331_channels,
+	.num_channels = ARRAY_SIZE(as7331_channels),
+};
+
 static const struct of_device_id as73211_of_match[] = {
-	{ .compatible = "ams,as73211" },
+	{ .compatible = "ams,as73211", &as73211_spec },
+	{ .compatible = "ams,as7331", &as7331_spec },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, as73211_of_match);
 
 static const struct i2c_device_id as73211_id[] = {
-	{ "as73211", 0 },
+	{ "as73211", (kernel_ulong_t)&as73211_spec },
+	{ "as7331", (kernel_ulong_t)&as7331_spec },
 	{ }
 };
 MODULE_DEVICE_TABLE(i2c, as73211_id);

-- 
2.39.2

Re: [PATCH v2 3/3] iio: light: as73211: add support for as7331

[Christian Eggers]

Hi Javier,

after being back from my extended Christmas holidays, I took a look
onto your changes:

On Wednesday, 3 January 2024, 13:08:53 CET, Javier Carrasco wrote:
> The AMS AS7331 is a UV light sensor with three channels: UVA, UVB and
> UVC (also known as deep UV and referenced as DUV in the iio core).
> Its internal structure and forming blocks are practically identical to
> the ones the AS73211 contains: API, internal DAC, I2C interface and
> registers, measurement modes, number of channels and pinout.
> 
> The only difference between them is the photodiodes used to acquire
> light, which means that only some modifications are required to add
> support for the AS7331 in the existing driver.
> 
> The temperature channel is identical for both devices and only the
> channel modifiers of the IIO_INTENSITY channels need to account for the
> device type.
> 
> The scale values have been obtained from the chapter "7.5 Transfer
> Function" of the official datasheet[1] for the configuration chosen as
> basis (Nclk = 1024 and GAIN = 1). Those values keep the units from the
> datasheet (nW/cm^2), as opposed to the units used for the AS73211
> (nW/m^2).
> 
> Add a new device-specific data structure to account for the device
> differences: channel types and scale of LSB per channel.
> 
> [1] https://ams.com/documents/20143/9106314/AS7331_DS001047_4-00.pdf
> 
> Tested-by: Christian Eggers <ceggers@arri.de>
> Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
> ---
>  drivers/iio/light/Kconfig   |   5 +-
>  drivers/iio/light/as73211.c | 141 ++++++++++++++++++++++++++++++++++++--------
>  2 files changed, 118 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig
> index 143003232d1c..fd5a9879a582 100644
> --- a/drivers/iio/light/Kconfig
> +++ b/drivers/iio/light/Kconfig
> @@ -87,13 +87,14 @@ config APDS9960
>  	  module will be called apds9960
>  
>  config AS73211
> -	tristate "AMS AS73211 XYZ color sensor"
> +	tristate "AMS AS73211 XYZ color sensor and AMS AS7331 UV sensor"
>  	depends on I2C
>  	select IIO_BUFFER
>  	select IIO_TRIGGERED_BUFFER
>  	help
>  	 If you say yes here you get support for the AMS AS73211
> -	 JENCOLOR(R) Digital XYZ Sensor.
> +	 JENCOLOR(R) Digital XYZ and the AMS AS7331 UVA, UVB and UVC
> +	 ultraviolet sensors.
>  
>  	 For triggered measurements, you will need an additional trigger driver
>  	 like IIO_HRTIMER_TRIGGER or IIO_SYSFS_TRIGGER.
> diff --git a/drivers/iio/light/as73211.c b/drivers/iio/light/as73211.c
> index b4c6f389a292..44daf816ae57 100644
> --- a/drivers/iio/light/as73211.c
> +++ b/drivers/iio/light/as73211.c
> @@ -1,6 +1,7 @@
>  // SPDX-License-Identifier: GPL-2.0-only
>  /*
> - * Support for AMS AS73211 JENCOLOR(R) Digital XYZ Sensor
> + * Support for AMS AS73211 JENCOLOR(R) Digital XYZ Sensor and AMS AS7331
> + * UVA, UVB and UVC (DUV) Ultraviolet Sensor
>   *
>   * Author: Christian Eggers <ceggers@arri.de>
>   *
> @@ -9,7 +10,9 @@
>   * Color light sensor with 16-bit channels for x, y, z and temperature);
>   * 7-bit I2C slave address 0x74 .. 0x77.
>   *
> - * Datasheet: https://ams.com/documents/20143/36005/AS73211_DS000556_3-01.pdf
> + * Datasheets:
> + * AS73211: https://ams.com/documents/20143/36005/AS73211_DS000556_3-01.pdf
> + * AS7331: https://ams.com/documents/20143/9106314/AS7331_DS001047_4-00.pdf
>   */
>  
>  #include <linux/bitfield.h>
> @@ -84,6 +87,20 @@ static const int as73211_hardwaregain_avail[] = {
>  	1, 2, 4, 8, 16, 32, 64, 128, 256, 512, 1024, 2048,
>  };
>  
> +struct as73211_data;
> +
> +/**
> + * struct spec_dev_data - device-specific data
> + * @intensity_scale:  Function to retrieve intensity scale values.
> + * @channel:          Device channels.
> + * @num_channels:     Number of channels of the device.
> + */
> +struct spec_dev_data {

I would call it as73211_spec_dev_data (is the C++ One Definition Rule relevant for
the kernel?)

> +	int (*intensity_scale)(struct as73211_data *data, int chan, int *val, int *val2);
> +	struct iio_chan_spec const *channel;
s/channel/channels/

> +	int num_channels;
> +};
> +
>  /**
>   * struct as73211_data - Instance data for one AS73211
>   * @client: I2C client.
> @@ -94,6 +111,7 @@ static const int as73211_hardwaregain_avail[] = {
>   * @mutex:  Keeps cached registers in sync with the device.
>   * @completion: Completion to wait for interrupt.
>   * @int_time_avail: Available integration times (depend on sampling frequency).
> + * @spec_dev: device-specific configuration.
>   */
>  struct as73211_data {
>  	struct i2c_client *client;
> @@ -104,6 +122,7 @@ struct as73211_data {
>  	struct mutex mutex;
>  	struct completion completion;
>  	int int_time_avail[AS73211_SAMPLE_TIME_NUM * 2];
> +	const struct spec_dev_data *spec_dev;
>  };
>  
>  #define AS73211_COLOR_CHANNEL(_color, _si, _addr) { \
> @@ -138,6 +157,10 @@ struct as73211_data {
>  #define AS73211_SCALE_Y 298384270  /* nW/m^2 */
>  #define AS73211_SCALE_Z 160241927  /* nW/m^2 */
>  
> +#define AS7331_SCALE_UVA 340000  /* nW/cm^2 */
> +#define AS7331_SCALE_UVB 378000  /* nW/cm^2 */
> +#define AS7331_SCALE_UVC 166000  /* nW/cm^2 */
> +
>  /* Channel order MUST match devices result register order */
>  #define AS73211_SCAN_INDEX_TEMP 0
>  #define AS73211_SCAN_INDEX_X    1
> @@ -176,6 +199,28 @@ static const struct iio_chan_spec as73211_channels[] = {
>  	IIO_CHAN_SOFT_TIMESTAMP(AS73211_SCAN_INDEX_TS),
>  };
>  
> +static const struct iio_chan_spec as7331_channels[] = {
> +	{
> +		.type = IIO_TEMP,
> +		.info_mask_separate =
> +			BIT(IIO_CHAN_INFO_RAW) |
> +			BIT(IIO_CHAN_INFO_OFFSET) |
> +			BIT(IIO_CHAN_INFO_SCALE),
> +		.address = AS73211_OUT_TEMP,
> +		.scan_index = AS73211_SCAN_INDEX_TEMP,
> +		.scan_type = {
> +			.sign = 'u',
> +			.realbits = 16,
> +			.storagebits = 16,
> +			.endianness = IIO_LE,
> +		}
> +	},
> +	AS73211_COLOR_CHANNEL(LIGHT_UVA, AS73211_SCAN_INDEX_X, AS73211_OUT_MRES1),
> +	AS73211_COLOR_CHANNEL(LIGHT_UVB, AS73211_SCAN_INDEX_Y, AS73211_OUT_MRES2),
> +	AS73211_COLOR_CHANNEL(LIGHT_DUV, AS73211_SCAN_INDEX_Z, AS73211_OUT_MRES3),
> +	IIO_CHAN_SOFT_TIMESTAMP(AS73211_SCAN_INDEX_TS),
> +};
> +
>  static unsigned int as73211_integration_time_1024cyc(struct as73211_data *data)
>  {
>  	/*
> @@ -316,6 +361,48 @@ static int as73211_req_data(struct as73211_data *data)
>  	return 0;
>  }
>  
> +static int as73211_intensity_scale(struct as73211_data *data, int chan,
> +				   int *val, int *val2)
> +{
> +	switch (chan) {
> +	case IIO_MOD_X:
> +		*val = AS73211_SCALE_X;
> +		break;
> +	case IIO_MOD_Y:
> +		*val = AS73211_SCALE_Y;
> +		break;
> +	case IIO_MOD_Z:
> +		*val = AS73211_SCALE_Z;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +	*val2 = as73211_integration_time_1024cyc(data) * as73211_gain(data);
> +
> +	return IIO_VAL_FRACTIONAL;
> +}
> +
> +static int as7331_intensity_scale(struct as73211_data *data, int chan,
> +				  int *val, int *val2)
> +{
> +	switch (chan) {
> +	case IIO_MOD_LIGHT_UVA:
> +		*val = AS7331_SCALE_UVA;
> +		break;
> +	case IIO_MOD_LIGHT_UVB:
> +		*val = AS7331_SCALE_UVB;
> +		break;
> +	case IIO_MOD_LIGHT_DUV:
> +		*val = AS7331_SCALE_UVC;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +	*val2 = as73211_integration_time_1024cyc(data) * as73211_gain(data);
> +
> +	return IIO_VAL_FRACTIONAL;
> +}
> +
>  static int as73211_read_raw(struct iio_dev *indio_dev, struct iio_chan_spec const *chan,
>  			     int *val, int *val2, long mask)
>  {
> @@ -355,29 +442,13 @@ static int as73211_read_raw(struct iio_dev *indio_dev, struct iio_chan_spec cons
>  			*val2 = AS73211_SCALE_TEMP_MICRO;
>  			return IIO_VAL_INT_PLUS_MICRO;
>  
> -		case IIO_INTENSITY: {
> -
> -			switch (chan->channel2) {
> -			case IIO_MOD_X:
> -				*val = AS73211_SCALE_X;
> -				break;
> -			case IIO_MOD_Y:
> -				*val = AS73211_SCALE_Y;
> -				break;
> -			case IIO_MOD_Z:
> -				*val = AS73211_SCALE_Z;
> -				break;
> -			default:
> -				return -EINVAL;
> -			}
> -			*val2 = as73211_integration_time_1024cyc(data) *
> -				as73211_gain(data);
> -
> -			return IIO_VAL_FRACTIONAL;
> +		case IIO_INTENSITY:
> +			return data->spec_dev->intensity_scale(data, chan->channel2,
> +							       val, val2);
>  
>  		default:
>  			return -EINVAL;
> -		}}
> +		}
>  
>  	case IIO_CHAN_INFO_SAMP_FREQ:
>  		/* f_samp is configured in CREG3 in powers of 2 (x 1.024 MHz) */
> @@ -675,13 +746,17 @@ static int as73211_probe(struct i2c_client *client)
>  	i2c_set_clientdata(client, indio_dev);
>  	data->client = client;
>  
> +	data->spec_dev = i2c_get_match_data(client);
> +	if (!data->spec_dev)
> +		return -EINVAL;
> +
>  	mutex_init(&data->mutex);
>  	init_completion(&data->completion);
>  
>  	indio_dev->info = &as73211_info;
>  	indio_dev->name = AS73211_DRV_NAME;
> -	indio_dev->channels = as73211_channels;
> -	indio_dev->num_channels = ARRAY_SIZE(as73211_channels);
> +	indio_dev->channels = data->spec_dev->channel;
> +	indio_dev->num_channels = data->spec_dev->num_channels;
>  	indio_dev->modes = INDIO_DIRECT_MODE;
>  
>  	ret = i2c_smbus_read_byte_data(data->client, AS73211_REG_OSR);
> @@ -771,14 +846,28 @@ static int as73211_resume(struct device *dev)
>  static DEFINE_SIMPLE_DEV_PM_OPS(as73211_pm_ops, as73211_suspend,
>  				as73211_resume);
>  
> +static const struct spec_dev_data as73211_spec = {
> +	.intensity_scale = as73211_intensity_scale,
> +	.channel = as73211_channels,
> +	.num_channels = ARRAY_SIZE(as73211_channels),
> +};
> +
> +static const struct spec_dev_data as7331_spec = {
> +	.intensity_scale = as7331_intensity_scale,
> +	.channel = as7331_channels,
> +	.num_channels = ARRAY_SIZE(as7331_channels),
> +};
> +
>  static const struct of_device_id as73211_of_match[] = {
> -	{ .compatible = "ams,as73211" },
> +	{ .compatible = "ams,as73211", &as73211_spec },
> +	{ .compatible = "ams,as7331", &as7331_spec },
>  	{ }
>  };
>  MODULE_DEVICE_TABLE(of, as73211_of_match);
>  
>  static const struct i2c_device_id as73211_id[] = {
> -	{ "as73211", 0 },
> +	{ "as73211", (kernel_ulong_t)&as73211_spec },
> +	{ "as7331", (kernel_ulong_t)&as7331_spec },
>  	{ }
>  };
>  MODULE_DEVICE_TABLE(i2c, as73211_id);
> 
> 

Tested-by: Christian Eggers <ceggers@arri.de>

Re: [PATCH v2 3/3] iio: light: as73211: add support for as7331

[Jonathan Cameron]

> > +struct as73211_data;
> > +
> > +/**
> > + * struct spec_dev_data - device-specific data
> > + * @intensity_scale:  Function to retrieve intensity scale values.
> > + * @channel:          Device channels.
> > + * @num_channels:     Number of channels of the device.
> > + */
> > +struct spec_dev_data {  
> 
> I would call it as73211_spec_dev_data (is the C++ One Definition Rule relevant for
> the kernel?)
> 
> > +	int (*intensity_scale)(struct as73211_data *data, int chan, int *val, int *val2);
> > +	struct iio_chan_spec const *channel;  
> s/channel/channels/
> 
> > +	int num_channels;
> > +};
Both good suggestions so I've applied them both and pushed out a new version of
the togreg tree.

Thanks,
Jonathan

Re: [PATCH v2 0/3] iio: light: add support for AMS AS7331

[Jonathan Cameron]

On Wed, 03 Jan 2024 13:08:50 +0100
Javier Carrasco <javier.carrasco.cruz@gmail.com> wrote:

> The AMS AS7331 UV light sensor measures three ultraviolet bands (UVA,
> UVB and UVC, also known as deep UV or DUV) as well as temperature.
> 
> This device is practically identical to the AMS AS73211 XYZ True Color
> sensor that is already supported by the iio subsystem, except for the
> photodiodes used to aquire the desired light wavelengths.
> 
> In order to reuse code and reduce maintenance load, this series extends
> the AS73211 driver to support the AS7331 as well.
> 
> Note that the UVA and UVB light modifiers have not been merged into the
> mainline kernel yet, but they are already available in Greg's char-misc
> git tree which can be found at
> git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git
> in the char-misc-next branch.
> 
> The original device AS73211 supported by the driver could only be tested
> briefly due to the lack of hardware. Instead, the i2c-stub module has
> been used to make sure that the driver registers the iio device properly
> and the attributes exported to sysfs are correct. Some basic register
> assignments reported the expected intensity scales and in principle
> nothing else should have been affected by the modifications in the code.
> 
> Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
Hi Javier,

Series applied - but given timing I'll only push this out as testing for
now as I'll want to rebase the IIO tree on rc1 once available in a couple of
weeks time.

Thanks,

Jonathan

> ---
> Changes in v2:
> - as73211.c: Use IIO_VAL_FRACTIONAL to retrieve scales of AS73211.
> - as73211.c: simplify device-specific data retrieval in probe function.
> - as73211.c: minor coding-style fix (shorter line).
> - Link to v1: https://lore.kernel.org/r/20231220-as7331-v1-0-745b73c27703@gmail.com
> 
> ---
> Javier Carrasco (3):
>       iio: light: as73211: use IIO_VAL_FRACTIONAL for intensity scales
>       dt-bindings: iio: light: as73211: add support for as7331
>       iio: light: as73211: add support for as7331
> 
>  .../devicetree/bindings/iio/light/ams,as73211.yaml |   7 +-
>  drivers/iio/light/Kconfig                          |   5 +-
>  drivers/iio/light/as73211.c                        | 142 +++++++++++++++++----
>  3 files changed, 123 insertions(+), 31 deletions(-)
> ---
> base-commit: e9215fcca2561b208c78359110ee4009b454f761
> change-id: 20231220-as7331-88a25ceeb66d
> 
> Best regards,

Re: [PATCH v2 0/3] iio: light: add support for AMS AS7331

[Javier Carrasco]

On 07.01.24 16:58, Jonathan Cameron wrote:
> On Wed, 03 Jan 2024 13:08:50 +0100
> Javier Carrasco <javier.carrasco.cruz@gmail.com> wrote:
> 
>> The AMS AS7331 UV light sensor measures three ultraviolet bands (UVA,
>> UVB and UVC, also known as deep UV or DUV) as well as temperature.
>>
>> This device is practically identical to the AMS AS73211 XYZ True Color
>> sensor that is already supported by the iio subsystem, except for the
>> photodiodes used to aquire the desired light wavelengths.
>>
>> In order to reuse code and reduce maintenance load, this series extends
>> the AS73211 driver to support the AS7331 as well.
>>
>> Note that the UVA and UVB light modifiers have not been merged into the
>> mainline kernel yet, but they are already available in Greg's char-misc
>> git tree which can be found at
>> git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git
>> in the char-misc-next branch.
>>
>> The original device AS73211 supported by the driver could only be tested
>> briefly due to the lack of hardware. Instead, the i2c-stub module has
>> been used to make sure that the driver registers the iio device properly
>> and the attributes exported to sysfs are correct. Some basic register
>> assignments reported the expected intensity scales and in principle
>> nothing else should have been affected by the modifications in the code.
>>
>> Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
> Hi Javier,
> 
> Series applied - but given timing I'll only push this out as testing for
> now as I'll want to rebase the IIO tree on rc1 once available in a couple of
> weeks time.
> 
> Thanks,
> 
> Jonathan
> 
Hi Jonathan,

I am happy with that approach. Some extra time to catch issues before
going live is a good thing anyway.

Thank you and best regards,

Javier Carrasco