2022-05-03 15:19:09

by Shreeya Patel

[permalink] [raw]
Subject: [PATCH v3 0/3] Add LTRF216A Driver

This patchset adds support for ltrf216a Ambient Light Sensor
and documents the DT bindings for the same.


Changes in v3
- Use u16 instead of u8 for int_time_fac
- Reorder headers in ltrf216a.c file
- Remove int_time_mapping table and use int_time_available
- Fix indentation in the bindings file.

Changes in v2
- Add support for 25ms and 50ms integration time.
- Rename some of the macros as per names given in datasheet
- Add a comment for the mutex lock
- Use read_avail callback instead of attributes and set the
appropriate _available bit.
- Use FIELD_PREP() at appropriate places.
- Add a constant lookup table for integration time and reg val
- Use BIT() macro for magic numbers.
- Improve error handling at few places.
- Use get_unaligned_le24() and div_u64()
- Use probe_new() callback and devm functions
- Return errors in probe using dev_err_probe()
- Use DEFINE_SIMPLE_DEV_PM_OPS()
- Correct the formula for lux to use 0.45 instead of 0.8
- Add interrupt and power supply property in DT bindings
- Add vendor prefix name as per the alphabetical order.


Shreeya Patel (3):
dt-bindings: vendor-prefixes: Add 'ltr' as deprecated vendor prefix
dt-bindings: Document ltrf216a light sensor bindings
iio: light: Add support for ltrf216a sensor

.../bindings/iio/light/liteon,ltrf216a.yaml | 51 +++
.../devicetree/bindings/vendor-prefixes.yaml | 3 +
drivers/iio/light/Kconfig | 10 +
drivers/iio/light/Makefile | 1 +
drivers/iio/light/ltrf216a.c | 343 ++++++++++++++++++
5 files changed, 408 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/light/liteon,ltrf216a.yaml
create mode 100644 drivers/iio/light/ltrf216a.c

--
2.30.2


2022-05-04 01:49:58

by Shreeya Patel

[permalink] [raw]
Subject: [PATCH v3 2/3] dt-bindings: Document ltrf216a light sensor bindings

Add devicetree bindings for ltrf216a ambient light sensor.

Reviewed-by: Krzysztof Kozlowski <[email protected]>
Signed-off-by: Shreeya Patel <[email protected]>
---

Changes in v3
- Fix indentation in the example section

Changes in v2
- Take over the maintainership for the bindings
- Add interrupt and power supply property in DT bindings

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

diff --git a/Documentation/devicetree/bindings/iio/light/liteon,ltrf216a.yaml b/Documentation/devicetree/bindings/iio/light/liteon,ltrf216a.yaml
new file mode 100644
index 000000000000..1389639cd7fd
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/light/liteon,ltrf216a.yaml
@@ -0,0 +1,51 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/light/liteon,ltrf216a.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: LTRF216A Ambient Light Sensor
+
+maintainers:
+ - Shreeya Patel <[email protected]>
+
+description:
+ Ambient light sensing with an i2c interface.
+
+properties:
+ compatible:
+ oneOf:
+ - const: liteon,ltrf216a
+ - const: ltr,ltrf216a
+ deprecated: true
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ vdd-supply:
+ description: Regulator that provides power to the sensor.
+
+required:
+ - compatible
+ - reg
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/irq.h>
+
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ light-sensor@53 {
+ compatible = "liteon,ltrf216a";
+ reg = <0x53>;
+ vdd-supply = <&vdd_regulator>;
+ interrupt-parent = <&gpio0>;
+ interrupts = <5 IRQ_TYPE_LEVEL_LOW>;
+ };
+ };
--
2.30.2

2022-05-04 04:27:54

by Shreeya Patel

[permalink] [raw]
Subject: [PATCH v3 1/3] dt-bindings: vendor-prefixes: Add 'ltr' as deprecated vendor prefix

'liteon' is the correct vendor prefix for devices released by
LITE-ON Technology Corp. But one of the released device which uses
ltr216a light sensor exposes the vendor prefix name as 'ltr' through
ACPI.

Hence, add 'ltr' as a deprecated vendor prefix which would suppress the
following warning in case the compatible string used in ltrf216a driver
is "ltr,ltrf216a"

WARNING: DT compatible string vendor "ltr" appears un-documented --
check ./Documentation/devicetree/bindings/vendor-prefixes.yaml
364: FILE: drivers/iio/light/ltrf216a.c:313:
+ { .compatible = "ltr,ltrf216a" },

Reviewed-by: Krzysztof Kozlowski <[email protected]>
Signed-off-by: Shreeya Patel <[email protected]>
---

Changes in v2
- Add vendor prefix name as per the alphabetical order.

Documentation/devicetree/bindings/vendor-prefixes.yaml | 3 +++
1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index 01430973ecec..02f94fba03b6 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -716,6 +716,9 @@ patternProperties:
description: Loongson Technology Corporation Limited
"^lsi,.*":
description: LSI Corp. (LSI Logic)
+ "^ltr,.*":
+ description: LITE-ON Technology Corp.
+ deprecated: true
"^lwn,.*":
description: Liebherr-Werk Nenzing GmbH
"^lxa,.*":
--
2.30.2

2022-05-04 10:43:06

by Shreeya Patel

[permalink] [raw]
Subject: [PATCH v3 3/3] iio: light: Add support for ltrf216a sensor

From: Zhigang Shi <[email protected]>

Add initial support for ltrf216a ambient light sensor.

Datasheet: gitlab.steamos.cloud/shreeya/iio/-/blob/main/LTRF216A.pdf
Co-developed-by: Shreeya Patel <[email protected]>
Signed-off-by: Shreeya Patel <[email protected]>
Signed-off-by: Zhigang Shi <[email protected]>
---

Changes in v3
- Use u16 instead of u8 for int_time_fac
- Reorder headers in ltrf216a.c file
- Remove int_time_mapping table and use int_time_available

Changes in v2
- Add support for 25ms and 50ms integration time.
- Rename some of the macros as per names given in datasheet
- Add a comment for the mutex lock
- Use read_avail callback instead of attributes and set the
appropriate _available bit.
- Use FIELD_PREP() at appropriate places.
- Add a constant lookup table for integration time and reg val
- Use BIT() macro for magic numbers.
- Improve error handling at few places.
- Use get_unaligned_le24() and div_u64()
- Use probe_new() callback and devm functions
- Return errors in probe using dev_err_probe()
- Use DEFINE_SIMPLE_DEV_PM_OPS()
- Correct the formula for lux to use 0.45 instead of 0.8


drivers/iio/light/Kconfig | 10 +
drivers/iio/light/Makefile | 1 +
drivers/iio/light/ltrf216a.c | 343 +++++++++++++++++++++++++++++++++++
3 files changed, 354 insertions(+)
create mode 100644 drivers/iio/light/ltrf216a.c

diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig
index a62c7b4b8678..33d2b24ba1da 100644
--- a/drivers/iio/light/Kconfig
+++ b/drivers/iio/light/Kconfig
@@ -318,6 +318,16 @@ config SENSORS_LM3533
changes. The ALS-control output values can be set per zone for the
three current output channels.

+config LTRF216A
+ tristate "Liteon LTRF216A Light Sensor"
+ depends on I2C
+ help
+ If you say Y or M here, you get support for Liteon LTRF216A
+ Ambient Light Sensor.
+
+ If built as a dynamically linked module, it will be called
+ ltrf216a.
+
config LTR501
tristate "LTR-501ALS-01 light sensor"
depends on I2C
diff --git a/drivers/iio/light/Makefile b/drivers/iio/light/Makefile
index d10912faf964..8fa91b9fe5b6 100644
--- a/drivers/iio/light/Makefile
+++ b/drivers/iio/light/Makefile
@@ -30,6 +30,7 @@ obj-$(CONFIG_SENSORS_ISL29028) += isl29028.o
obj-$(CONFIG_ISL29125) += isl29125.o
obj-$(CONFIG_JSA1212) += jsa1212.o
obj-$(CONFIG_SENSORS_LM3533) += lm3533-als.o
+obj-$(CONFIG_LTRF216A) += ltrf216a.o
obj-$(CONFIG_LTR501) += ltr501.o
obj-$(CONFIG_LV0104CS) += lv0104cs.o
obj-$(CONFIG_MAX44000) += max44000.o
diff --git a/drivers/iio/light/ltrf216a.c b/drivers/iio/light/ltrf216a.c
new file mode 100644
index 000000000000..1ad1eb4a4c6d
--- /dev/null
+++ b/drivers/iio/light/ltrf216a.c
@@ -0,0 +1,343 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * LTRF216A Ambient Light Sensor
+ *
+ * Copyright (C) 2021 Lite-On Technology Corp (Singapore)
+ * Author: Shi Zhigang <[email protected]>
+ *
+ * IIO driver for LTRF216A (7-bit I2C slave address 0x53).
+ */
+
+#include <linux/bitfield.h>
+#include <linux/delay.h>
+#include <linux/i2c.h>
+#include <linux/init.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/pm.h>
+#include <linux/iio/iio.h>
+#include <asm/unaligned.h>
+
+#define LTRF216A_DRV_NAME "ltrf216a"
+
+#define LTRF216A_MAIN_CTRL 0x00
+
+#define LTRF216A_ALS_DATA_STATUS BIT(3)
+#define LTRF216A_ALS_ENABLE_MASK BIT(1)
+
+#define LTRF216A_ALS_MEAS_RES 0x04
+#define LTRF216A_MAIN_STATUS 0x07
+#define LTRF216A_CLEAR_DATA_0 0x0A
+
+#define LTRF216A_ALS_DATA_0 0x0D
+
+static const int ltrf216a_int_time_available[5][2] = {
+ {0, 400000},
+ {0, 200000},
+ {0, 100000},
+ {0, 50000},
+ {0, 25000},
+};
+
+static const int ltrf216a_int_time_reg[5][2] = {
+ {400, 0x03},
+ {200, 0x13},
+ {100, 0x22},
+ {50, 0x31},
+ {25, 0x40},
+};
+
+struct ltrf216a_data {
+ struct i2c_client *client;
+ u32 int_time;
+ u16 int_time_fac;
+ u8 als_gain_fac;
+ struct mutex mutex; /* Protect read and write operations */
+};
+
+/* open air. need to update based on TP transmission rate. */
+#define WIN_FAC 1
+
+static const struct iio_chan_spec ltrf216a_channels[] = {
+ {
+ .type = IIO_LIGHT,
+ .info_mask_separate =
+ BIT(IIO_CHAN_INFO_PROCESSED) |
+ BIT(IIO_CHAN_INFO_INT_TIME),
+ .info_mask_separate_available =
+ BIT(IIO_CHAN_INFO_INT_TIME),
+ }
+};
+
+static int ltrf216a_init(struct iio_dev *indio_dev)
+{
+ int ret;
+ struct ltrf216a_data *data = iio_priv(indio_dev);
+
+ ret = i2c_smbus_read_byte_data(data->client, LTRF216A_MAIN_CTRL);
+ if (ret < 0) {
+ dev_err(&data->client->dev, "Error reading LTRF216A_MAIN_CTRL\n");
+ return ret;
+ }
+
+ /* enable sensor */
+ ret |= FIELD_PREP(LTRF216A_ALS_ENABLE_MASK, 1);
+ ret = i2c_smbus_write_byte_data(data->client, LTRF216A_MAIN_CTRL, ret);
+ if (ret < 0) {
+ dev_err(&data->client->dev, "Error writing LTRF216A_MAIN_CTRL\n");
+ return ret;
+ }
+
+ return 0;
+}
+
+static int ltrf216a_disable(struct iio_dev *indio_dev)
+{
+ int ret;
+ struct ltrf216a_data *data = iio_priv(indio_dev);
+
+ ret = i2c_smbus_write_byte_data(data->client, LTRF216A_MAIN_CTRL, 0);
+ if (ret < 0)
+ dev_err(&data->client->dev, "Error writing LTRF216A_MAIN_CTRL\n");
+
+ return ret;
+}
+
+static void als_ltrf216a_disable(void *data)
+{
+ struct iio_dev *indio_dev = data;
+
+ ltrf216a_disable(indio_dev);
+}
+
+static int ltrf216a_set_int_time(struct ltrf216a_data *data, int itime)
+{
+ int i, ret, index = -1;
+ u8 reg_val;
+
+ for (i = 0; i < ARRAY_SIZE(ltrf216a_int_time_available); i++) {
+ if (ltrf216a_int_time_available[i][1] == itime) {
+ index = i;
+ break;
+ }
+ }
+
+ if (index < 0)
+ return -EINVAL;
+
+ reg_val = ltrf216a_int_time_reg[index][1];
+ data->int_time_fac = ltrf216a_int_time_reg[index][0];
+
+ ret = i2c_smbus_write_byte_data(data->client, LTRF216A_ALS_MEAS_RES, reg_val);
+ if (ret < 0)
+ return ret;
+
+ data->int_time = itime;
+
+ return 0;
+}
+
+static int ltrf216a_get_int_time(struct ltrf216a_data *data, int *val, int *val2)
+{
+ *val = 0;
+ *val2 = data->int_time;
+ return IIO_VAL_INT_PLUS_MICRO;
+}
+
+static int ltrf216a_read_data(struct ltrf216a_data *data, u8 addr)
+{
+ int i, ret = -1, tries = 25;
+ u8 buf[3];
+
+ while (tries--) {
+ ret = i2c_smbus_read_byte_data(data->client, LTRF216A_MAIN_STATUS);
+ if (ret < 0)
+ return ret;
+ if (ret & LTRF216A_ALS_DATA_STATUS)
+ break;
+ msleep(20);
+ }
+
+ for (i = 0; i < 3; i++) {
+ ret = i2c_smbus_read_byte_data(data->client, addr);
+ if (ret < 0)
+ return ret;
+ buf[i] = ret;
+ addr++;
+ }
+
+ return get_unaligned_le24(&buf[0]);
+}
+
+static int ltrf216a_get_lux(struct ltrf216a_data *data)
+{
+ int greendata, cleardata;
+ u64 lux, div;
+
+ greendata = ltrf216a_read_data(data, LTRF216A_ALS_DATA_0);
+ cleardata = ltrf216a_read_data(data, LTRF216A_CLEAR_DATA_0);
+
+ if (greendata < 0 || cleardata < 0)
+ return -EINVAL;
+
+ lux = greendata * 45 * WIN_FAC * 100;
+ div = data->als_gain_fac * data->int_time_fac * 100;
+
+ return div_u64(lux, div);
+}
+
+static int ltrf216a_read_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan, int *val,
+ int *val2, long mask)
+{
+ int ret;
+ struct ltrf216a_data *data = iio_priv(indio_dev);
+
+ mutex_lock(&data->mutex);
+
+ switch (mask) {
+ case IIO_CHAN_INFO_PROCESSED:
+ ret = ltrf216a_get_lux(data);
+ if (ret < 0)
+ return ret;
+ *val = ret;
+ ret = IIO_VAL_INT;
+ break;
+ case IIO_CHAN_INFO_INT_TIME:
+ ret = ltrf216a_get_int_time(data, val, val2);
+ break;
+ default:
+ ret = -EINVAL;
+ }
+
+ mutex_unlock(&data->mutex);
+
+ return ret;
+}
+
+static int ltrf216a_write_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan, int val,
+ int val2, long mask)
+{
+ struct ltrf216a_data *data = iio_priv(indio_dev);
+ int ret;
+
+ switch (mask) {
+ case IIO_CHAN_INFO_INT_TIME:
+ if (val != 0)
+ return -EINVAL;
+ mutex_lock(&data->mutex);
+ ret = ltrf216a_set_int_time(data, val2);
+ mutex_unlock(&data->mutex);
+ return ret;
+ default:
+ return -EINVAL;
+ }
+}
+
+static int ltrf216a_read_available(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ const int **vals, int *type, int *length,
+ long mask)
+{
+ switch (mask) {
+ case IIO_CHAN_INFO_INT_TIME:
+ *length = ARRAY_SIZE(ltrf216a_int_time_available) * 2;
+ *vals = (const int *)ltrf216a_int_time_available;
+ *type = IIO_VAL_INT_PLUS_MICRO;
+ return IIO_AVAIL_LIST;
+ default:
+ return -EINVAL;
+ }
+}
+
+static const struct iio_info ltrf216a_info = {
+ .read_raw = ltrf216a_read_raw,
+ .write_raw = ltrf216a_write_raw,
+ .read_avail = ltrf216a_read_available,
+};
+
+static int ltrf216a_probe(struct i2c_client *client)
+{
+ struct ltrf216a_data *data;
+ struct iio_dev *indio_dev;
+ int ret;
+
+ indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
+ if (!indio_dev)
+ return -ENOMEM;
+
+ data = iio_priv(indio_dev);
+ i2c_set_clientdata(client, indio_dev);
+ data->client = client;
+
+ mutex_init(&data->mutex);
+
+ indio_dev->info = &ltrf216a_info;
+ indio_dev->name = LTRF216A_DRV_NAME;
+ indio_dev->channels = ltrf216a_channels;
+ indio_dev->num_channels = ARRAY_SIZE(ltrf216a_channels);
+ indio_dev->modes = INDIO_DIRECT_MODE;
+
+ ret = ltrf216a_init(indio_dev);
+ if (ret < 0)
+ return dev_err_probe(&client->dev, ret,
+ "ltrf216a chip init failed\n");
+
+ data->int_time = 100000;
+ data->int_time_fac = 100;
+ data->als_gain_fac = 3;
+
+ ret = devm_add_action_or_reset(&client->dev, als_ltrf216a_disable,
+ indio_dev);
+ if (ret < 0)
+ return ret;
+
+ return devm_iio_device_register(&client->dev, indio_dev);
+}
+
+static int ltrf216a_suspend(struct device *dev)
+{
+ struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
+
+ return ltrf216a_disable(indio_dev);
+}
+
+static int ltrf216a_resume(struct device *dev)
+{
+ struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
+
+ return ltrf216a_init(indio_dev);
+}
+
+static DEFINE_SIMPLE_DEV_PM_OPS(ltrf216a_pm_ops, ltrf216a_suspend, ltrf216a_resume);
+
+static const struct i2c_device_id ltrf216a_id[] = {
+ { LTRF216A_DRV_NAME, 0 },
+ {}
+};
+MODULE_DEVICE_TABLE(i2c, ltrf216a_id);
+
+static const struct of_device_id ltrf216a_of_match[] = {
+ { .compatible = "liteon,ltrf216a", },
+ { .compatible = "ltr,ltrf216a", },
+ {}
+};
+MODULE_DEVICE_TABLE(of, ltrf216a_of_match);
+
+static struct i2c_driver ltrf216a_driver = {
+ .driver = {
+ .name = LTRF216A_DRV_NAME,
+ .pm = pm_sleep_ptr(&ltrf216a_pm_ops),
+ .of_match_table = ltrf216a_of_match,
+ },
+ .probe_new = ltrf216a_probe,
+ .id_table = ltrf216a_id,
+};
+
+module_i2c_driver(ltrf216a_driver);
+
+MODULE_AUTHOR("Shi Zhigang <[email protected]>");
+MODULE_DESCRIPTION("LTRF216A ambient light sensor driver");
+MODULE_LICENSE("GPL");
--
2.30.2


2022-05-09 04:53:37

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v3 3/3] iio: light: Add support for ltrf216a sensor

On Tue, 3 May 2022 20:13:54 +0530
Shreeya Patel <[email protected]> wrote:

> From: Zhigang Shi <[email protected]>
>
> Add initial support for ltrf216a ambient light sensor.
>
> Datasheet: gitlab.steamos.cloud/shreeya/iio/-/blob/main/LTRF216A.pdf
> Co-developed-by: Shreeya Patel <[email protected]>
> Signed-off-by: Shreeya Patel <[email protected]>
> Signed-off-by: Zhigang Shi <[email protected]>

One locking bug, otherwise just the lock documentation to resolve.

Thanks,

Jonathan



> +
> +struct ltrf216a_data {
> + struct i2c_client *client;
> + u32 int_time;
> + u16 int_time_fac;
> + u8 als_gain_fac;
> + struct mutex mutex; /* Protect read and write operations */

See other branch of thread for feedback on that comment

Thanks for highlighting that btw. I'd forgotten about it so might have
missed it on a fresh read through.

> +};
> +
> +/* open air. need to update based on TP transmission rate. */
> +#define WIN_FAC 1
> +

> +
> +static int ltrf216a_read_data(struct ltrf216a_data *data, u8 addr)
> +{
> + int i, ret = -1, tries = 25;
> + u8 buf[3];
> +
> + while (tries--) {
> + ret = i2c_smbus_read_byte_data(data->client, LTRF216A_MAIN_STATUS);
> + if (ret < 0)
> + return ret;
> + if (ret & LTRF216A_ALS_DATA_STATUS)
> + break;
> + msleep(20);
> + }
> +
> + for (i = 0; i < 3; i++) {
> + ret = i2c_smbus_read_byte_data(data->client, addr);

Might be worth seeing if the device copes with
i2c_smbus_read_i2c_block_data()
The datasheet doesn't mention it though so it might well not work.


> + if (ret < 0)
> + return ret;
> + buf[i] = ret;
> + addr++;
> + }
> +
> + return get_unaligned_le24(&buf[0]);
> +}

> +
> +static int ltrf216a_read_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan, int *val,
> + int *val2, long mask)
> +{
> + int ret;
> + struct ltrf216a_data *data = iio_priv(indio_dev);
> +
> + mutex_lock(&data->mutex);
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_PROCESSED:
> + ret = ltrf216a_get_lux(data);
> + if (ret < 0)
> + return ret;

Check your locking. This path doesn't unlock the mutex.

> + *val = ret;
> + ret = IIO_VAL_INT;
> + break;
> + case IIO_CHAN_INFO_INT_TIME:
> + ret = ltrf216a_get_int_time(data, val, val2);
> + break;
> + default:
> + ret = -EINVAL;
> + }
> +
> + mutex_unlock(&data->mutex);
> +
> + return ret;
> +}
> +

> +