On 11/21/23 05:10, Marek Vasut wrote:
The ISL76682 is very basic ALS which only supports ALS or IR mode
in four ranges, 1k/4k/16k/64k LUX. There is no IRQ support or any
other fancy functionality.
Signed-off-by: Marek Vasut <marex@xxxxxxx>
---
Cc: Alexander Stein <alexander.stein@xxxxxxxxxxxxxxx>
Cc: Andre Werner <andre.werner@xxxxxxxxxxxxxxxxxxxxx>
Cc: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
Cc: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>
Cc: Conor Dooley <conor+dt@xxxxxxxxxx>
Cc: Fabio Estevam <festevam@xxxxxxx>
Cc: Guenter Roeck <linux@xxxxxxxxxxxx>
Cc: Jonathan Cameron <jic23@xxxxxxxxxx>
Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@xxxxxxxxxx>
Cc: Lars-Peter Clausen <lars@xxxxxxxxxx>
Cc: Luca Ceresoli <luca.ceresoli@xxxxxxxxxxx>
Cc: Mark Brown <broonie@xxxxxxxxxx>
Cc: Matti Vaittinen <mazziesaccount@xxxxxxxxx>
Cc: Naresh Solanki <naresh.solanki@xxxxxxxxxxxxx>
Cc: Patrick Rudolph <patrick.rudolph@xxxxxxxxxxxxx>
Cc: Rob Herring <robh+dt@xxxxxxxxxx>
Cc: Stefan Windfeldt-Prytz <stefan.windfeldt-prytz@xxxxxxxx>
Cc: Vincent Tremblay <vincent@xxxxxxxxxxxxx>
Cc: devicetree@xxxxxxxxxxxxxxx
Cc: linux-iio@xxxxxxxxxxxxxxx
---
V2: - Overhaul the driver
- Cache the entire 8-bit command register instead of parts of it
and build up and rewrite the entire register if necessary
- Fix illumination scale, add intensity scale, add integration time
V3: - Limit the read data to 16bit ADC range
- Update Kconfig description
- Update macros, drop bitshifts
- Switch over to table lookup for lux ranges
- Switch over to .read_avail instead of attributes
- Use guard where applicable
- Drop remove function in favor of reset action
V4: - Address feedback from Andy
- Add missing includes
- Change ISL76682_ADC_MAX to BIT(16) - 1
- Drop initial ret assignment in isl76682_read_raw()
- Move return -EINVAL to default: switch-case branch
- Use switch-case consistenly instead of if/else
- Drop trailing commas
- Add comment to isl76682_clear_configure_reg on command zeroing on failure
- Drop i2c_set_clientdata
- Update devm_regmap_init_i2c return value handling
---
drivers/iio/light/Kconfig | 15 ++
drivers/iio/light/Makefile | 1 +
drivers/iio/light/isl76682.c | 364 +++++++++++++++++++++++++++++++++++
3 files changed, 380 insertions(+)
create mode 100644 drivers/iio/light/isl76682.c
diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig
index 45edba797e4c7..9e8cdc091556d 100644
--- a/drivers/iio/light/Kconfig
+++ b/drivers/iio/light/Kconfig
@@ -252,6 +252,21 @@ config ISL29125
To compile this driver as a module, choose M here: the module will be
called isl29125.
+config ISL76682
+ tristate "Intersil ISL76682 Light Sensor"
+ depends on I2C
+ select REGMAP_I2C
+ help
+ Say Y here if you want to build a driver for the Intersil ISL76682
+ Ambient Light Sensor and IR Intensity sensor. This driver provides
+ the readouts via standard IIO sysfs and device interface. Both ALS
+ illuminance and IR illuminance are provided raw with separate scale
+ setting which can be configured via sysfs, the default scale is 1000
+ lux, other options are 4000/16000/64000 lux.
+
+ To compile this driver as a module, choose M here: the module will be
+ called isl76682.
+
config HID_SENSOR_ALS
depends on HID_SENSOR_HUB
select IIO_BUFFER
diff --git a/drivers/iio/light/Makefile b/drivers/iio/light/Makefile
index c0db4c4c36ec9..09fa585f3109f 100644
--- a/drivers/iio/light/Makefile
+++ b/drivers/iio/light/Makefile
@@ -28,6 +28,7 @@ obj-$(CONFIG_IQS621_ALS) += iqs621-als.o
obj-$(CONFIG_SENSORS_ISL29018) += isl29018.o
obj-$(CONFIG_SENSORS_ISL29028) += isl29028.o
obj-$(CONFIG_ISL29125) += isl29125.o
+obj-$(CONFIG_ISL76682) += isl76682.o
obj-$(CONFIG_JSA1212) += jsa1212.o
obj-$(CONFIG_SENSORS_LM3533) += lm3533-als.o
obj-$(CONFIG_LTR501) += ltr501.o
diff --git a/drivers/iio/light/isl76682.c b/drivers/iio/light/isl76682.c
new file mode 100644
index 0000000000000..7f0ccd0d37539
--- /dev/null
+++ b/drivers/iio/light/isl76682.c
@@ -0,0 +1,364 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * IIO driver for the light sensor ISL76682.
+ * ISL76682 is Ambient Light Sensor
+ *
+ * Copyright (c) 2023 Marek Vasut <marex@xxxxxxx>
+ */
+
+#include <linux/array_size.h>
+#include <linux/bits.h>
+#include <linux/cleanup.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/regmap.h>
+#include <linux/types.h>
+
+#include <linux/iio/iio.h>
+
+#define ISL76682_REG_COMMAND 0x00
+
+#define ISL76682_COMMAND_EN BIT(7)
+#define ISL76682_COMMAND_MODE_CONTINUOUS BIT(6)
+#define ISL76682_COMMAND_LIGHT_IR BIT(5)
+
+#define ISL76682_COMMAND_RANGE_LUX_1K 0x0
+#define ISL76682_COMMAND_RANGE_LUX_4K 0x1
+#define ISL76682_COMMAND_RANGE_LUX_16K 0x2
+#define ISL76682_COMMAND_RANGE_LUX_64K 0x3
+#define ISL76682_COMMAND_RANGE_LUX_MASK GENMASK(1, 0)
+
+#define ISL76682_REG_ALSIR_L 0x01
+
+#define ISL76682_REG_ALSIR_U 0x02
+
+#define ISL76682_NUM_REGS (ISL76682_REG_ALSIR_U + 1)
+
+#define ISL76682_CONV_TIME_MS 100
+#define ISL76682_INT_TIME_US 90000
+
+#define ISL76682_ADC_MAX (BIT(16) - 1)
+
+struct isl76682_chip {
+ /*
+ * Lock to synchronize access to device command register
+ * and the content of range variable below.
+ */
+ struct mutex lock;
+ struct regmap *regmap;
+ u8 range;
+ u8 command;
+};
+
+struct isl76682_range {
+ u8 range;
+ u32 als;
+ u32 ir;
+};
+
+static struct isl76682_range isl76682_range_table[] = {
+ { ISL76682_COMMAND_RANGE_LUX_1K, 15000, 10500 },
+ { ISL76682_COMMAND_RANGE_LUX_4K, 60000, 42000 },
+ { ISL76682_COMMAND_RANGE_LUX_16K, 240000, 168000 },
+ { ISL76682_COMMAND_RANGE_LUX_64K, 960000, 673000 }
+};
+
+static int isl76682_get(struct isl76682_chip *chip, bool mode_ir, int *data)
+{
+ u8 command;
+ int ret;
+
+ command = ISL76682_COMMAND_EN | ISL76682_COMMAND_MODE_CONTINUOUS |
+ chip->range;
+
+ if (mode_ir)
+ command |= ISL76682_COMMAND_LIGHT_IR;
+
+ if (command != chip->command) {
+ ret = regmap_write(chip->regmap, ISL76682_REG_COMMAND, command);
+ if (ret)
+ return ret;
+
+ /* Need to wait for conversion time if ALS/IR mode enabled */
+ msleep(ISL76682_CONV_TIME_MS);
+
+ chip->command = command;
+ }
+
+ ret = regmap_bulk_read(chip->regmap, ISL76682_REG_ALSIR_L, data, 2);
+ *data &= ISL76682_ADC_MAX;
+ return ret;
+}
+
+static int isl76682_write_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int val, int val2, long mask)
+{
+ struct isl76682_chip *chip = iio_priv(indio_dev);
+ int i;
+
+ if (chan->type != IIO_LIGHT && chan->type != IIO_INTENSITY)
+ return -EINVAL;
+
+ if (mask != IIO_CHAN_INFO_SCALE)
+ return -EINVAL;
+
+ if (val != 0)
+ return -EINVAL;
+
+ for (i = 0; i < ARRAY_SIZE(isl76682_range_table); i++) {
+ if (chan->type == IIO_LIGHT) {
+ if (val2 != isl76682_range_table[i].als)
+ continue;
+ } else if (chan->type == IIO_INTENSITY) {
+ if (val2 != isl76682_range_table[i].ir)
+ continue;
+ }
I like this table-based look-up for write (and read) of scales. Looking
at this I see an analogy to some of the regulator stuff, like for
example the ramp-up values. What I do very much like in the regulator
subsystem is the drivers/regulator/helpers.c
I wonder if similar approach would be usable in IIO as well? I mean,
providing readily written iio_regmap_read/write_raw_<functionality>()
and iio_available_*() helpers for the simple devices where we just have
value-register mapping? I mean, driver would just populate something like:
struct iio_scale_desc {
int *scale_val_table;
int *scale_val2_table;
int num_scales;
int scale_reg_addr;
int scale_reg_mask;
};
and call helper like
int iio_regmap_read_raw_scale(struct iio_dev *idev,
struct iio_scale_desc *sd, int *val,
int *val2)"
provided by IIO framework.
Similar helper for writing new scales and getting available scales.
Later this could be expanded by allowing specifying the type of provided
values (in the example case, IIO_VAL_INT_PLUS_x - but maybe this would
be extensible (and worth) to support also the other options?)
I know it's a bit much to be done in the context of this series. Hence I
am definitely not insisting this to be done here! OTOH, the embedded
Linux is not in EU next year so maybe Marek would forgive me before we
meet next time :pondering:
Anyways - does this sound like a sensible thing to do? I guess it could
help simplifying some drivers a little.
Oh. Only after writing of this I noticed the range is written in HW only
together with the 'start' command. I guess this is how the IC operates -
you need to write all configs together with starting the measurement? Or
is that just an optimization to avoid extra writes? If it's the first,
then a suggested iio_regmap_*() -helper wouldn't work here. I might've
added a comment explaining why range is written in isl76682_get() and
not here to the isl76682_get().
Anyways - this driver looks good to me. (What a long way of saying that).
Reviewed-by: Matti Vaittinen <mazziesaccount@xxxxxxxxx>
+
+ scoped_guard(mutex, &chip->lock)
+ chip->range = isl76682_range_table[i].range;
+ return 0;
+ }
+
+ return -EINVAL;
+}
+
Yours,
-- Matti
--
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland
~~ When things go utterly wrong vim users can always type :help! ~~
