Hi dee Ho Marek,
Long time no chat :/
On 11/16/23 15:13, 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
---
NOTE: I am not 100% sure about the SCALE handling, can you please
check esp. that one ? Thanks !
I checked the data-sheet. Please correct me if I am wrong - I did only a
quick read.
To me it seems there are 16bit LSB data registers and 4 supported scales.
I assume the data register value 0 means measurement was not done - but
rest of the values are computed as if the 0 would mean 0 Lx - and
register values then grow linearly from that up-to the range max.
Hence, the most sensitive range is from 0 Lx to 1000 Lx. The full 16bit
value being 1000 Lx. For this range, the scaling should thus be
1000 / 0xFFFF (1000 being decimal).
+enum isl76682_als_ir_mode {
+ ISL76682_MODE_NONE = 0,
+ ISL76682_MODE_ALS,
+ ISL76682_MODE_IR,
+};
+
+struct isl76682_chip {
+ struct mutex lock;
+ struct regmap *regmap;
+ enum isl76682_als_ir_mode als_ir_mode;
+ int lux_scale;
+};
+
+static int isl76682_set_als_scale(struct isl76682_chip *chip, int lux_scale)
+{
+ int ret, val;
+
Would renaming the lux_scale to lux_range make it clearer? I think the
scale is really lux_scale/MAX_DATA_REG_VAL
+ if (lux_scale == 1000)
+ val = ISL76682_COMMAND_RANGE_LUX_1K;
+ else if (lux_scale == 4000)
+ val = ISL76682_COMMAND_RANGE_LUX_4K;
+ else if (lux_scale == 16000)
+ val = ISL76682_COMMAND_RANGE_LUX_16K;
+ else if (lux_scale == 64000)
+ val = ISL76682_COMMAND_RANGE_LUX_64K;
+ else
+ return -EINVAL;
+
+ ret = regmap_update_bits(chip->regmap, ISL76682_REG_COMMAND,
+ ISL76682_COMMAND_RANGE_LUX_MASK, val);
+ if (ret < 0)
+ return ret;
+
+ chip->lux_scale = lux_scale;
Maybe leave this to be cached by the regmap?
+
+ return 0;
+}
+
+static int isl76682_set_als_ir_mode(struct isl76682_chip *chip,
+ enum isl76682_als_ir_mode mode)
+{
+ int ret;
+
+ if (chip->als_ir_mode == mode)
+ return 0;
+
+ if (mode == ISL76682_MODE_NONE) {
+ return regmap_clear_bits(chip->regmap, ISL76682_REG_COMMAND,
+ ISL76682_COMMAND_EN);
+ }
+
+ ret = isl76682_set_als_scale(chip, chip->lux_scale);
+ if (ret < 0)
+ return ret;
Does the HW require the scale to be set here?
+
+ if (mode == ISL76682_MODE_ALS) {
+ ret = regmap_clear_bits(chip->regmap, ISL76682_REG_COMMAND,
+ ISL76682_COMMAND_LIGHT_IR);
+ } else {
+ ret = regmap_set_bits(chip->regmap, ISL76682_REG_COMMAND,
+ ISL76682_COMMAND_LIGHT_IR);
+ }
+ if (ret < 0)
+ return ret;
+
+ /* Enable the ALS/IR */
+ ret = regmap_set_bits(chip->regmap, ISL76682_REG_COMMAND,
+ ISL76682_COMMAND_EN |
+ ISL76682_COMMAND_MODE_CONTINUOUS);
+ if (ret < 0)
+ return ret;
+
+ /* Need to wait for conversion time if ALS/IR mode enabled */
+ msleep(ISL76682_CONV_TIME_MS);
+
+ chip->als_ir_mode = mode;
+
+ return 0;
+}
+
+static int isl76682_read_als_ir(struct isl76682_chip *chip, int *als_ir)
+{
+ unsigned int lsb, msb;
+ int ret;
+
+ ret = regmap_read(chip->regmap, ISL76682_REG_ALSIR_L, &lsb);
+ if (ret < 0)
+ return ret;
+
+ ret = regmap_read(chip->regmap, ISL76682_REG_ALSIR_U, &msb);
+ if (ret < 0)
+ return ret;
+
+ *als_ir = (msb << 8) | lsb;
+
+ return 0;
+}
+
+static int isl76682_als_get(struct isl76682_chip *chip, int *als_data)
+{
+ int als_ir_data;
+ int ret;
+
+ ret = isl76682_set_als_ir_mode(chip, ISL76682_MODE_ALS);
+ if (ret < 0)
+ return ret;
+
+ ret = isl76682_read_als_ir(chip, &als_ir_data);
+ if (ret < 0)
+ return ret;
+
+ *als_data = als_ir_data;
+
+ return 0;
+}
+
+static int isl76682_ir_get(struct isl76682_chip *chip, int *ir_data)
+{
+ int ret;
+
+ ret = isl76682_set_als_ir_mode(chip, ISL76682_MODE_IR);
+ if (ret < 0)
+ return ret;
+
+ return isl76682_read_als_ir(chip, ir_data);
+}
+
+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 ret;
+
+ if (chan->type != IIO_LIGHT)
+ return -EINVAL;
+
+ if (mask != IIO_CHAN_INFO_SCALE)
+ return -EINVAL;
+
+ mutex_lock(&chip->lock);
+ ret = isl76682_set_als_scale(chip, val);
+ mutex_unlock(&chip->lock);
This looks a bit odd to me. I was under impression that the values would
by default be IIO_VAL_INT_PLUS_MICRO unless the format is given in
iio_info struct. If so, I'd expect the val to be zero for all of the
scales because all scales are smaller than 1. (I may be wrong though).
Eg, I'd expect that when range 1000 is set (scale 1000 / 65535), val =
0, val1 roughly 15259 (to mean 0.015259).
+
+ return ret;
+}
+
+static int isl76682_read_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 ret;
+
+ mutex_lock(&chip->lock);
+
+ ret = -EINVAL;
+ switch (mask) {
+ case IIO_CHAN_INFO_RAW:
+ switch (chan->type) {
+ case IIO_LIGHT:
+ ret = isl76682_als_get(chip, val);
+ break;
+ case IIO_INTENSITY:
+ ret = isl76682_ir_get(chip, val);
+ break;
+ default:
+ break;
+ }
+
+ if (ret < 0)
+ break;
+
+ ret = IIO_VAL_INT;
+ break;
+ case IIO_CHAN_INFO_SCALE:
+ if (chan->type != IIO_LIGHT)
+ break;
+ *val = chip->lux_scale;
+ *val2 = ISL76682_ADC_MAX;
+ ret = IIO_VAL_FRACTIONAL;
This looks correct to me (with the remark about maybe leaving the
caching of scale to regmap).
+ break;
+ default:
+ break;
+ }
+
+ mutex_unlock(&chip->lock);
+
+ return ret;
+}
+
+static IIO_CONST_ATTR(in_illuminance_scale_available, "1000 4000 16000 64000");
This does not look correct to me. I'd guess the correct available scales
should be 0.015259... 0.06... 0.24... 0.96... (up to used accuracy - I
might go with the IIO_VAL_INT_PLUS_NANO. And, I didn't compute other
than the first, just stole the rest from the data-sheet MIN range value
which should equal to ADC count 1).
Please just ignore me if you think I am writing nonsense - wouldn't be
the first time!
Yours,
-- Matti
--
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland
~~ When things go utterly wrong vim users can always type :help! ~~
