dt-bindings: iio: light: add ltr390

Implements driver for the Ambient/UV Light sensor LTR390. The driver exposes two ways of getting sensor readings:
1. Raw UV Counts directly from the sensor
2. The computed UV Index value with a percision of 2 decimal places

NOTE: Ambient light sensing has not been implemented yet.

Datasheet:
https://optoelectronics.liteon.com/upload/download/DS86-2015-0004/LTR-390UV_Final_%20DS_V1%201.pdf

Driver tested on RPi Zero 2W

Signed-off-by Anshul Dalal <[email protected]>

drivers/iio/light/ltr390.c new file +5 -0

new file mode 100644
index 000000000000..67ca028ce828
@@ -0,0 +1,232 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * IIO driver for Lite-On LTR390 ALS and UV sensor
+ * (7-bit I2C slave address 0x53)
+ *

drivers/iio/light/ltr390.c new file +32 -0

+#include <linux/delay.h>
+#include <linux/i2c.h>
+#include <linux/iio/iio.h>
+#include <linux/math.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/regmap.h>
+
+#define LTR390_DEVICE_NAME	    "ltr390"
+
+#define LTR390_MAIN_CTRL	    0x00
+#define LTR390_PART_ID		    0x06
+#define LTR390_UVS_DATA		    0x10
+
+#define LTR390_SW_RESET		    BIT(4)
+#define LTR390_UVS_MODE		    BIT(3)
+#define LTR390_SENSOR_ENABLE	    BIT(1)
+
+#define LTR390_PART_NUMBER_ID	    0xb
+#define LTR390_FRACTIONAL_PERCISION 100
+
+/*
+ * At 20-bit resolution (integration time: 400ms) and 18x gain, 2300 counts of
+ * the sensor are equal to 1 UV Index [Datasheet Page#8].
+ *
+ * For the default resolution of 18-bit (integration time: 100ms) and default
+ * gain of 3x, the counts/uvi are calculated as follows:
+ * 2300 / ((3/18) * (100/400)) = 95.83
+ */
+#define LTR390_COUNTS_PER_UVI 96
+
+/*

drivers/iio/light/ltr390.c new file +6 -0

+};
+
+static const struct regmap_config ltr390_regmap_config = {
+	.name = LTR390_DEVICE_NAME,
+	.reg_bits = 8,
+	.reg_stride = 1,

drivers/iio/light/ltr390.c new file +15 -0

+
+	ret = regmap_bulk_read(data->regmap, register_address, recieve_buffer,
+			       sizeof(recieve_buffer));
+	if (ret) {
+		dev_err(dev, "failed to read measurement data: %d\n", ret);
+		mutex_unlock(&data->lock);
+		return ret;
+	}
+
+	mutex_unlock(&data->lock);
+	return get_unaligned_le24(recieve_buffer);
+}
+
+static int ltr390_get_uv_index(struct ltr390_data *data)
+{

drivers/iio/light/ltr390.c new file +22 -0

+
+	return uv_index;
+}
+
 
+
+static const struct iio_chan_spec ltr390_channels[] = {
+	{
+		.type = IIO_UVINDEX,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED)
+	},
+	{
+		.type = IIO_INTENSITY,
+		.channel2 = IIO_MOD_LIGHT_UV,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW)
+	},
+};
+
+static int ltr390_probe(struct i2c_client *client)
+{
+	struct ltr390_data *data;
+	struct iio_dev *indio_dev;
+	int ret, part_number;

drivers/iio/light/ltr390.c new file +4 -0

+		return -ENODEV;
+	}
+	dev_dbg(&client->dev, "LTR390, product id: 0x%x\n", part_number);
+

drivers/iio/light/ltr390.c new file +5 -0

+module_i2c_driver(ltr390_driver);
+
+MODULE_AUTHOR("Anshul Dalal <[email protected]>");
+MODULE_DESCRIPTION("Lite-On LTR390 ALS and UV sensor Driver");
+MODULE_LICENSE("GPL");

Quoted 5 lines · On Fri, 17 Nov 2023 13:15:53 +0530 Anshul Dalal <[email protected]> wrote:

Implements driver for the Ambient/UV Light sensor LTR390. The driver exposes two ways of getting sensor readings:
1. Raw UV Counts directly from the sensor
2. The computed UV Index value with a percision of 2 decimal places

Quoted 4 lines

NOTE: Ambient light sensing has not been implemented yet.

Datasheet:
https://optoelectronics.liteon.com/upload/download/DS86-2015-0004/LTR-390UV_Final_%20DS_V1%201.pdf

Make this a formal Datasheet tag, just before the Signed-off-by below

Quoted 4 lines

Driver tested on RPi Zero 2W

Signed-off-by Anshul Dalal <[email protected]>

Hi Anshul,

Some comments on this one inline. Some of these overlap with comments on the other drivers you've submitted. Normally I'd moan about sending too many drivers at a time, but fair enough given you sent them out over a couple of weeks and just happened to hit time when I was travelling.

My main question in here is why have the two channels - conversion looks linear so you should be fine exposing IIO_CHAN_INFO_RAW + IIO_CHAN_INFO_SCALE on a single channel and leaving userspace to do the maths.

Jonathan

Quoted 11 lines

drivers/iio/light/ltr390.c new file +5 -0

new file mode 100644
index 000000000000..67ca028ce828
@@ -0,0 +1,232 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * IIO driver for Lite-On LTR390 ALS and UV sensor
+ * (7-bit I2C slave address 0x53)
+ *

drivers/iio/light/ltr390.c line 15

Quoted 10 lines

drivers/iio/light/ltr390.c new file +10 -0

+ * Based on the work of:
+ *   Shreeya Patel and Shi Zhigang (LTRF216 Driver)
+ *
+ * Copyright (C) 2023 Anshul Dalal <[email protected]>
+ *
+ * Datasheet:
+ *   https://optoelectronics.liteon.com/upload/download/DS86-2015-0004/LTR-390UV_Final_%20DS_V1%201.pdf
+ *
+ * TODO:
+ *   - Support for configurable gain and resolution

Not using PROCESSED will help with this, so I'd drop that even in this initial driver.

drivers/iio/light/ltr390.c line 21

Quoted 7 lines

Quoted 1 line

+ * - Sensor suspend/resume support

+ * - Add support for reading the ALS
+ * - Interrupt support

+ */

drivers/iio/light/ltr390.c new file +2 -0

+
+#include <asm/unaligned.h>

Put this in a block of includes after the linux ones.

Quoted 39 lines

drivers/iio/light/ltr390.c new file +39 -0

+#include <linux/delay.h>
+#include <linux/i2c.h>
+#include <linux/iio/iio.h>
+#include <linux/math.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/regmap.h>
+
+#define LTR390_DEVICE_NAME	    "ltr390"
+
+#define LTR390_MAIN_CTRL	    0x00
+#define LTR390_PART_ID		    0x06
+#define LTR390_UVS_DATA		    0x10
+
+#define LTR390_SW_RESET		    BIT(4)
+#define LTR390_UVS_MODE		    BIT(3)
+#define LTR390_SENSOR_ENABLE	    BIT(1)
+
+#define LTR390_PART_NUMBER_ID	    0xb
+#define LTR390_FRACTIONAL_PERCISION 100
+
+/*
+ * At 20-bit resolution (integration time: 400ms) and 18x gain, 2300 counts of
+ * the sensor are equal to 1 UV Index [Datasheet Page#8].
+ *
+ * For the default resolution of 18-bit (integration time: 100ms) and default
+ * gain of 3x, the counts/uvi are calculated as follows:
+ * 2300 / ((3/18) * (100/400)) = 95.83
+ */
+#define LTR390_COUNTS_PER_UVI 96
+
+/*
+ * Window Factor is needed when the device is under Window glass with coated
+ * tinted ink. This is to compensate for the light loss due to the lower
+ * transmission rate of the window glass and helps * in calculating lux.
+ */
+#define LTR390_WINDOW_FACTOR 1
+
+struct ltr390_data {

drivers/iio/light/ltr390.c line 63

Quoted 3 lines

drivers/iio/light/ltr390.c new file +3 -0

+	struct regmap *regmap;
+	struct i2c_client *client;
+	struct mutex lock;

All locks need a comment explaining the scope of data they protect. Note that regmap and the i2c bus will have their own locks by default so I'm not sure you need one here at all as I'm not seeing read modify write cycles or anything like that (I might be missing one though!)

Quoted 6 lines

drivers/iio/light/ltr390.c new file +6 -0

+};
+
+static const struct regmap_config ltr390_regmap_config = {
+	.name = LTR390_DEVICE_NAME,
+	.reg_bits = 8,
+	.reg_stride = 1,

drivers/iio/light/ltr390.c line 79

Quoted 10 lines

drivers/iio/light/ltr390.c new file +10 -0

+	.val_bits = 8,
+};
+
+static int ltr390_register_read(struct ltr390_data *data, u8 register_address)
+{
+	struct device *dev = &data->client->dev;
+	int ret;
+	u8 recieve_buffer[3];
+
+	mutex_lock(&data->lock);

guard(mutex)(&data->lock);
here then you can just return directly without worrying about manual unlocking of the mutex. This stuff is fairly new in the kernel but very useful for cases like this! I have a set of driver cleanup that I'll send out that does this in a few IIO drivers, once I've caught up with reviews etc.

This isn't a critical path where we have to care that we will then unlock after the maths to extract value in the final line of the function.

Quoted 15 lines

drivers/iio/light/ltr390.c new file +15 -0

+
+	ret = regmap_bulk_read(data->regmap, register_address, recieve_buffer,
+			       sizeof(recieve_buffer));
+	if (ret) {
+		dev_err(dev, "failed to read measurement data: %d\n", ret);
+		mutex_unlock(&data->lock);
+		return ret;
+	}
+
+	mutex_unlock(&data->lock);
+	return get_unaligned_le24(recieve_buffer);
+}
+
+static int ltr390_get_uv_index(struct ltr390_data *data)
+{

drivers/iio/light/ltr390.c line 104

Quoted 10 lines

drivers/iio/light/ltr390.c new file +10 -0

+	int ret;
+	int uv_index;
+
+	ret = ltr390_register_read(data, LTR390_UVS_DATA);
+	if (ret < 0)
+		return ret;
+
+	uv_index = DIV_ROUND_CLOSEST(ret * LTR390_FRACTIONAL_PERCISION *
+					     LTR390_WINDOW_FACTOR,
+				     LTR390_COUNTS_PER_UVI);

This looks like a linear conversion. Perhaps make it a userspace problem as division much easier in userspace where floating point is available.

Quoted 23 lines

drivers/iio/light/ltr390.c new file +22 -0

+
+	return uv_index;
+}
+
 
+
+static const struct iio_chan_spec ltr390_channels[] = {
+	{
+		.type = IIO_UVINDEX,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED)
+	},
+	{
+		.type = IIO_INTENSITY,
+		.channel2 = IIO_MOD_LIGHT_UV,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW)
+	},
+};
+
+static int ltr390_probe(struct i2c_client *client)
+{
+	struct ltr390_data *data;
+	struct iio_dev *indio_dev;
+	int ret, part_number;

drivers/iio/light/ltr390.c line 165

Quoted 10 lines

drivers/iio/light/ltr390.c new file +10 -0

+
+	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	data = iio_priv(indio_dev);
+
+	data->regmap = devm_regmap_init_i2c(client, &ltr390_regmap_config);
+	if (IS_ERR(data->regmap))
+		return dev_err_probe(&client->dev, PTR_ERR(data->regmap),

There are quite a few &client->dev in here. I'd introduce struct device *dev = &client->dev;
as a local variable then use that to shorten all those lines a little.

drivers/iio/light/ltr390.c line 169

Quoted 4 lines

drivers/iio/light/ltr390.c new file +4 -0

+				     "regmap initialization failed\n");
+
+	data->client = client;
+	i2c_set_clientdata(client, indio_dev);

Why set this? I don' think you are using it.

Quoted 1 line

[..]

Quoted 10 lines

drivers/iio/light/ltr390.c line 63

Quoted 5 lines

drivers/iio/light/ltr390.c new file +4 -0

+struct ltr390_data {
+	struct regmap *regmap;
+	struct i2c_client *client;
+	struct mutex lock;  

All locks need a comment explaining the scope of data they protect. Note that regmap and the i2c bus will have their own locks by default so I'm not sure you need one here at all as I'm not seeing read modify write cycles or anything like that (I might be missing one though!)

My goal with the mutex was to protect the sysfs though that might be unnecessary.

Quoted 31 lines

Quoted 17 lines

drivers/iio/light/ltr390.c new file +14 -0

+};
+
+static const struct regmap_config ltr390_regmap_config = {
+	.name = LTR390_DEVICE_NAME,
+	.reg_bits = 8,
+	.reg_stride = 1,
+	.val_bits = 8,
+};
+
 
 
+static int ltr390_probe(struct i2c_client *client)
+{
+	struct ltr390_data *data;
+	struct iio_dev *indio_dev;
+	int ret, part_number;

drivers/iio/light/ltr390.c

Quoted 10 lines

drivers/iio/light/ltr390.c new file +10 -0

+
+	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	data = iio_priv(indio_dev);
+
+	data->regmap = devm_regmap_init_i2c(client, &ltr390_regmap_config);
+	if (IS_ERR(data->regmap))
+		return dev_err_probe(&client->dev, PTR_ERR(data->regmap),  

There are quite a few &client->dev in here. I'd introduce struct device *dev = &client->dev;
as a local variable then use that to shorten all those lines a little.

Quoted 7 lines

drivers/iio/light/ltr390.c line 169

Quoted 4 lines

drivers/iio/light/ltr390.c new file +4 -0

+				     "regmap initialization failed\n");
+
+	data->client = client;
+	i2c_set_clientdata(client, indio_dev);  

Why set this? I don' think you are using it.

It seems to be necessary for regmap to work properly, I tested without it and I get an EREMOTEIO(121) when reading the part id.

Quoted 2 lines

Quoted 2 lines

[..]

Thanks for the review,
Best regards,
Anshul

Add binding for Lite-On LTR390 which is an Ambient/UV light sensor that communicates over i2c with an address of 0x53.

Datasheet:
https://optoelectronics.liteon.com/upload/download/DS86-2015-0004/LTR-390UV_Final_%20DS_V1%201.pdf

Signed-off-by Anshul Dalal <[email protected]>

---

Add binding for Lite-On LTR390 which is an Ambient/UV light sensor that communicates over i2c with an address of 0x53.

Datasheet:
https://optoelectronics.liteon.com/upload/download/DS86-2015-0004/LTR-390UV_Final_%20DS_V1%201.pdf

Signed-off-by Anshul Dalal <[email protected]>

---

Changes for v2:
- Added missing properties in the example

---
 .../bindings/iio/light/liteon,ltr390.yaml     | 56 +++++++++++++++++++
 1 file changed, 56 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/light/liteon,ltr390.yaml

Documentation/devicetree/bindings/iio/light/liteon,ltr390.yaml new file +28 -0

new file mode 100644
index 000000000000..5d98ef2af74d
@@ -0,0 +1,56 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/light/liteon,ltr390.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Lite-On LTR390 ALS and UV Sensor
+
+description: |
+  The Lite-On LTR390 is an ALS (Ambient Light Sensor) and a UV sensor in a
+  single package with i2c address of 0x53.
+
+  Datasheet:
+    https://optoelectronics.liteon.com/upload/download/DS86-2015-0004/LTR-390UV_Final_%20DS_V1%201.pdf
+
+maintainers:
+  - Anshul Dalal <[email protected]>
+
+properties:
+  compatible:
+    enum:
+      - liteon,ltr390
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1

Documentation/devicetree/bindings/iio/light/liteon,ltr390.yaml new file +18 -0

+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        light-sensor@53 {
+            compatible = "liteon,ltr390";
+            reg = <0x53>;
+            interrupts = <18 IRQ_TYPE_EDGE_FALLING>;
+            vdd-supply = <&vdd_regulator>;
+        };
+    };