On Thu, 24 May 2018 11:18:30 +0200 Borys Movchan <borys.movchan@xxxxxxxx> wrote: > From: Borys Movchan <borysmn@xxxxxxxx> > > Add Intersil ISL29033 Ultra-Low Lux, Low Power, Integrated > Digital Ambient Light Sensor driver. > > The ISL29033 is an integrated ambient and infrared > light-to-digital converter. Its advanced, self-calibrated photodiode > array emulates human eye response with excellent IR rejection. The > on-chip 16-bit ADC is capable of rejecting 50Hz and 60Hz > flicker caused by artificial light sources. The lux range select > feature allows users to program the lux range for optimized > counts/lux. > > datasheet: https://www.intersil.com/content/dam/intersil/documents/isl2/isl29033.pdf > > Signed-off-by: Borys Movchan <borys.movchan@xxxxxxxx> Hi Boryz, Various comments inline. The biggest one is that we tend to avoid having to have userspace figure out explicit power control (it is hard for it to know what needs doing). The enable stuff is there really for counting channels where we want to say 'count from now'. The ideal is to use the runtime pm framework to let the device autosleep when it is not being used and bring it up automatically when it is. Here things are a little more complex as switching channels requires a mode change - not much we can do about that - just switch on demand. Jonathan > --- > .../devicetree/bindings/iio/light/isl29033.txt | 17 + > drivers/iio/light/Kconfig | 9 + > drivers/iio/light/Makefile | 1 + > drivers/iio/light/isl29033.c | 766 +++++++++++++++++++++ > 4 files changed, 793 insertions(+) > create mode 100644 Documentation/devicetree/bindings/iio/light/isl29033.txt > create mode 100644 drivers/iio/light/isl29033.c > > diff --git a/Documentation/devicetree/bindings/iio/light/isl29033.txt b/Documentation/devicetree/bindings/iio/light/isl29033.txt > new file mode 100644 > index 000000000000..41870aeacebf > --- /dev/null > +++ b/Documentation/devicetree/bindings/iio/light/isl29033.txt > @@ -0,0 +1,17 @@ Bindings need to be in a separate patch (to make it easy for the devicetree people to find them). The devicetree list and maintainers must be cc'd on all new bindings (I haven't done it here as there are some issues I'll point out so just cc them on v4). > +ISL29033 Ultra low lux ambient light sensor > + > +Required properties: > + - compatible : Must be "isil,isl29033". > + - reg: the I2C address of the device > + > +Optional properties: > + - rext: Rext resistor nominal (in kOhm) Name should include: * Vendor specific prefix - this isn't a general thing * Units of the value. Some explanation of what this resistor is for would also be helpful. > + > +Example: > + > + als_sensor@44 { > + compatible = "isil,isl29033"; > + reg = <0x44>; > + rext = <1000>; > + }; > + > diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig > index 074e50657366..2d845618e6b0 100644 > --- a/drivers/iio/light/Kconfig > +++ b/drivers/iio/light/Kconfig > @@ -182,6 +182,15 @@ config SENSORS_ISL29028 > Proximity value via iio. The ISL29028 provides the concurrent sensing > of ambient light and proximity. > > +config SENSORS_ISL29033 SENSORS_ doesn't add anything (and has come from HWMON most likely). For new drivers don't put it there. > + tristate "Intersil ISL29033 Ultra-Low Lux, Low Power, Integrated Digital Ambient Light Sensor" > + depends on I2C > + select REGMAP_I2C > + help > + Provides driver for the Intersil's ISL29033 device. > + This driver supports the sysfs interface to get the ALS and IR intensity > + value via IIO. > + > config ISL29125 > tristate "Intersil ISL29125 digital color light sensor" > depends on I2C > diff --git a/drivers/iio/light/Makefile b/drivers/iio/light/Makefile > index f1777036d4f8..98f5f1f0a45a 100644 > --- a/drivers/iio/light/Makefile > +++ b/drivers/iio/light/Makefile > @@ -22,6 +22,7 @@ obj-$(CONFIG_HID_SENSOR_ALS) += hid-sensor-als.o > obj-$(CONFIG_HID_SENSOR_PROX) += hid-sensor-prox.o > obj-$(CONFIG_SENSORS_ISL29018) += isl29018.o > obj-$(CONFIG_SENSORS_ISL29028) += isl29028.o > +obj-$(CONFIG_SENSORS_ISL29033) += isl29033.o > obj-$(CONFIG_ISL29125) += isl29125.o > obj-$(CONFIG_JSA1212) += jsa1212.o > obj-$(CONFIG_SENSORS_LM3533) += lm3533-als.o > diff --git a/drivers/iio/light/isl29033.c b/drivers/iio/light/isl29033.c > new file mode 100644 > index 000000000000..8b38fd718c1f > --- /dev/null > +++ b/drivers/iio/light/isl29033.c > @@ -0,0 +1,766 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * isl29033.c: A iio driver for the light sensor ISL 29033. > + * > + * IIO driver for monitoring ambient light intensity in lux and infrared > + * sensing. > + * > + * Copyright (c) 2018, Axis Communication AB > + * Author: Borys Movchan <Borys.Movchan@xxxxxxxx> > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + * > + * This program is distributed in the hope that it will be useful, but WITHOUT > + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or > + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for > + * more details. It's normal now to drop the explicit statement of the GPL in favour of just SPDX tag + copyright / description. > + */ > + > +#include <linux/module.h> > +#include <asm/div64.h> > +#include <linux/i2c.h> > +#include <linux/err.h> > +#include <linux/mutex.h> > +#include <linux/delay.h> > +#include <linux/regmap.h> > +#include <linux/slab.h> > +#include <linux/iio/iio.h> > +#include <linux/iio/sysfs.h> > +#include <linux/acpi.h> Alphabetical headers preferred. > + > +#define ISL29033_REG_ADD_COMMAND1 0x00 > +#define ISL29033_CMD1_OPMODE_SHIFT 5 > +#define ISL29033_CMD1_OPMODE_MASK (7 << ISL29033_CMD1_OPMODE_SHIFT) > +#define ISL29033_CMD1_OPMODE_POWER_DOWN 0 > +#define ISL29033_CMD1_OPMODE_ALS_CONT 5 > +#define ISL29033_CMD1_OPMODE_IR_CONT 6 > + > +#define ISL29033_REG_ADD_COMMAND2 0x01 > +#define ISL29033_CMD2_RESOLUTION_SHIFT 2 > +#define ISL29033_CMD2_RESOLUTION_MASK (0x3 << ISL29033_CMD2_RESOLUTION_SHIFT) > + > +#define ISL29033_CMD2_RANGE_SHIFT 0 > +#define ISL29033_CMD2_RANGE_MASK (0x3 << ISL29033_CMD2_RANGE_SHIFT) > + > +#define ISL29033_CMD2_SCHEME_SHIFT 7 > +#define ISL29033_CMD2_SCHEME_MASK (0x1 << ISL29033_CMD2_SCHEME_SHIFT) > + > +#define ISL29033_REG_ADD_DATA_LSB 0x02 > +#define ISL29033_REG_ADD_DATA_MSB 0x03 > + > +#define ISL29033_REG_TEST 0x08 > +#define ISL29033_TEST_SHIFT 0 > +#define ISL29033_TEST_MASK (0xFF << ISL29033_TEST_SHIFT) > + > +#define ISL29033_REF_REXT 499 /* In kOhm */ > + > +#define ISL29033_MICRO 1000000 > + > +enum isl29033_int_time { > + ISL29033_INT_TIME_16, > + ISL29033_INT_TIME_12, > + ISL29033_INT_TIME_8, > + ISL29033_INT_TIME_4, > +}; > + > +#define isl29033_int_utime(adcmax) \ > + ((unsigned int)(adcmax) * (ISL29033_MICRO / 1000) / 655) > + > +static const unsigned int isl29033_int_utimes[4] = { > + isl29033_int_utime(65536), > + isl29033_int_utime(4096), > + isl29033_int_utime(256), > + isl29033_int_utime(16) > +}; > + > +#define isl29033_mkscale(range, adcmax) \ > + { (range) / (adcmax), \ > + ((unsigned int)(range) * (ISL29033_MICRO / 10) \ > + / (adcmax) * 10) % ISL29033_MICRO } > + > +static const struct isl29033_scale { > + unsigned int scale; > + unsigned int uscale; > +} isl29033_scales[4][4] = { > + { > + isl29033_mkscale(125, 65536), isl29033_mkscale(500, 65536), > + isl29033_mkscale(2000, 65536), isl29033_mkscale(8000, 65536) > + }, > + { > + isl29033_mkscale(125, 4096), isl29033_mkscale(500, 4096), > + isl29033_mkscale(2000, 4096), isl29033_mkscale(8000, 4096) > + }, > + { > + isl29033_mkscale(125, 256), isl29033_mkscale(500, 256), > + isl29033_mkscale(2000, 256), isl29033_mkscale(8000, 256) > + }, > + { > + isl29033_mkscale(125, 16), isl29033_mkscale(500, 16), > + isl29033_mkscale(2000, 16), isl29033_mkscale(8000, 16) > + } > +}; > + one blank line is enough. > + > +struct isl29033_chip { > + struct regmap *regmap; > + struct mutex lock; > + unsigned int int_time; > + unsigned int calibscale; > + unsigned int ucalibscale; > + struct isl29033_scale scale; > + unsigned int rext; > + unsigned int opmode; > + bool suspended; > + blank line doesn't add anything. > +}; > + > +#define isl29033_rext_normalize(chip, val) \ > + ((val) * ISL29033_REF_REXT / (chip)->rext) > + > +static unsigned int isl29033_rext_normalize2(struct isl29033_chip *chip, > + unsigned int val, unsigned int val2) > +{ > + val = val - isl29033_rext_normalize(chip, val) > + * chip->rext / ISL29033_REF_REXT; > + val2 = (val2 + val * ISL29033_MICRO) * ISL29033_REF_REXT / chip->rext; > + return val2; > +} > + > +static int isl29033_set_integration_time(struct isl29033_chip *chip, > + unsigned int utime) > +{ > + unsigned int i; > + int ret; > + unsigned int int_time, new_int_time; > + > + for (i = 0; i < ARRAY_SIZE(isl29033_int_utimes); ++i) { > + if (utime == isl29033_int_utimes[i] > + * chip->rext / ISL29033_REF_REXT) { > + new_int_time = i; > + break; > + } > + } > + > + if (i >= ARRAY_SIZE(isl29033_int_utimes)) > + return -EINVAL; > + > + ret = regmap_update_bits(chip->regmap, ISL29033_REG_ADD_COMMAND2, > + ISL29033_CMD2_RESOLUTION_MASK, > + i << ISL29033_CMD2_RESOLUTION_SHIFT); > + if (ret < 0) > + return ret; > + > + /* Keep the same range when integration time changes */ > + int_time = chip->int_time; > + for (i = 0; i < ARRAY_SIZE(isl29033_scales[int_time]); ++i) { > + if (chip->scale.scale == isl29033_scales[int_time][i].scale && > + chip->scale.uscale == isl29033_scales[int_time][i].uscale) { > + chip->scale = isl29033_scales[new_int_time][i]; > + break; > + } > + } > + chip->int_time = new_int_time; > + > + return 0; > +} > + > +static int isl29033_set_scale(struct isl29033_chip *chip, int scale, int uscale) > +{ > + unsigned int i; > + int ret; > + struct isl29033_scale new_scale; > + > + for (i = 0; i < ARRAY_SIZE(isl29033_scales[chip->int_time]); ++i) { > + if (scale == > + isl29033_rext_normalize(chip, > + isl29033_scales[chip->int_time][i].scale) > + && uscale == > + isl29033_rext_normalize2(chip, > + isl29033_scales[chip->int_time][i].scale, > + isl29033_scales[chip->int_time][i].uscale)) { > + new_scale = isl29033_scales[chip->int_time][i]; > + break; > + } > + } > + > + if (i >= ARRAY_SIZE(isl29033_scales[chip->int_time])) > + return -EINVAL; > + > + ret = regmap_update_bits(chip->regmap, ISL29033_REG_ADD_COMMAND2, > + ISL29033_CMD2_RANGE_MASK, > + i << ISL29033_CMD2_RANGE_SHIFT); > + if (ret < 0) > + return ret; > + > + chip->scale = new_scale; > + > + return 0; > +} > + > +static int isl29033_set_mode(struct isl29033_chip *chip, int mode) > +{ > + > + int ret; > + > + ret = regmap_update_bits(chip->regmap, ISL29033_REG_ADD_COMMAND1, > + ISL29033_CMD1_OPMODE_MASK, > + mode << ISL29033_CMD1_OPMODE_SHIFT); > + if (ret < 0) { > + struct device *dev = regmap_get_device(chip->regmap); > + > + dev_err(dev, > + "Error in setting operating mode with err %d\n", ret); > + return ret; > + } > + > + if (chip->int_time < 20000) > + usleep_range(chip->int_time, chip->int_time * 2); > + else > + msleep(chip->int_time / 1000); > + > + return 0; > +} > + > +static int isl29033_read_sensor_input(struct isl29033_chip *chip) > +{ > + int ret; > + unsigned int lsb; > + unsigned int msb; > + struct device *dev = regmap_get_device(chip->regmap); > + > + if (chip->opmode == ISL29033_CMD1_OPMODE_POWER_DOWN) > + return -EBUSY; > + regmap_bulk_read? Then you can put it directly in an appropriate endianness of variable and possible save some maths. > + ret = regmap_read(chip->regmap, ISL29033_REG_ADD_DATA_LSB, &lsb); > + if (ret < 0) { > + dev_err(dev, > + "Error in reading LSB DATA with err %d\n", ret); > + return ret; > + } > + > + ret = regmap_read(chip->regmap, ISL29033_REG_ADD_DATA_MSB, &msb); > + if (ret < 0) { > + dev_err(dev, > + "Error in reading MSB DATA with err %d\n", ret); > + return ret; > + } > + dev_vdbg(dev, "MSB 0x%x and LSB 0x%x\n", msb, lsb); > + > + return (msb << 8) | lsb; > +} > + > +static int isl29033_read_lux(struct isl29033_chip *chip, int *lux, int *ulux) > +{ > + int ret; > + unsigned int tmpres; > + > + ret = isl29033_read_sensor_input(chip); > + if (ret < 0) > + return ret; > + > + ret++; > + > + tmpres = ret * isl29033_rext_normalize2(chip, chip->scale.scale, > + chip->scale.uscale); > + *lux = ret * isl29033_rext_normalize(chip, chip->scale.scale) > + + tmpres / ISL29033_MICRO; > + *ulux = tmpres % ISL29033_MICRO; > + > + *lux = *lux * chip->calibscale; > + *ulux = *ulux * chip->calibscale + *lux * chip->ucalibscale > + + *ulux * chip->ucalibscale / ISL29033_MICRO; > + > + return IIO_VAL_INT_PLUS_MICRO; > +} > + > +static int isl29033_read_ir(struct isl29033_chip *chip, int *ir) > +{ > + int ret; > + > + ret = isl29033_read_sensor_input(chip); > + if (ret < 0) > + return ret; > + > + *ir = ret; > + > + return IIO_VAL_INT; > +} > + > +static ssize_t isl29033_in_illuminance_scale_available > + (struct device *dev, struct device_attribute *attr, > + char *buf) > +{ > + struct iio_dev *indio_dev = dev_to_iio_dev(dev); > + struct isl29033_chip *chip = iio_priv(indio_dev); > + unsigned int i; > + int len = 0; > + > + mutex_lock(&chip->lock); > + for (i = 0; i < ARRAY_SIZE(isl29033_scales[chip->int_time]); ++i) > + len += sprintf(buf + len, "%d.%06d ", > + isl29033_rext_normalize(chip, > + isl29033_scales[chip->int_time][i].scale), > + isl29033_rext_normalize2(chip, > + isl29033_scales[chip->int_time][i].scale, > + isl29033_scales[chip->int_time][i].uscale)); > + mutex_unlock(&chip->lock); > + > + buf[len - 1] = '\n'; > + > + return len; > +} > + > +static ssize_t isl29033_in_illuminance_integration_time_available > + (struct device *dev, struct device_attribute *attr, > + char *buf) > +{ > + struct iio_dev *indio_dev = dev_to_iio_dev(dev); > + struct isl29033_chip *chip = iio_priv(indio_dev); > + unsigned int i; > + int len = 0; > + > + for (i = 0; i < ARRAY_SIZE(isl29033_int_utimes); ++i) > + len += sprintf(buf + len, "0.%06d ", > + isl29033_int_utimes[i] > + * chip->rext / ISL29033_REF_REXT); > + > + buf[len - 1] = '\n'; > + > + return len; > +} > + > +static int isl29033_write_raw(struct iio_dev *indio_dev, > + struct iio_chan_spec const *chan, > + int val, > + int val2, > + long mask) > +{ > + struct isl29033_chip *chip = iio_priv(indio_dev); > + int ret = -EINVAL; > + > + mutex_lock(&chip->lock); > + if (chip->suspended) { > + ret = -EBUSY; > + goto write_done; > + } > + switch (mask) { > + case IIO_CHAN_INFO_CALIBSCALE: > + if (chan->type == IIO_LIGHT) { > + chip->calibscale = val; > + chip->ucalibscale = val2; > + ret = 0; > + } > + break; > + case IIO_CHAN_INFO_INT_TIME: > + if (chan->type == IIO_LIGHT && !val) > + ret = isl29033_set_integration_time(chip, val2); > + break; > + case IIO_CHAN_INFO_SCALE: > + if (chan->type == IIO_LIGHT) > + ret = isl29033_set_scale(chip, val, val2); > + break; > + case IIO_CHAN_INFO_ENABLE: Hmm. Generally the enable isn't used simply to change device modes. Most userspace code simply won't touch it as it is perfectly possible to workout when the device should be enabled from other controls, such as on the read. Runtime power management with auto sleeping is very much preferred. Saves on userspace having to be clever. This isn't a fast device so most of the time it'll be fine if we do a 'best guess' on whether to be powered up or not. > + if (val) > + switch (chan->type) { > + case IIO_LIGHT: > + chip->opmode = ISL29033_CMD1_OPMODE_ALS_CONT; > + break; > + case IIO_INTENSITY: > + chip->opmode = ISL29033_CMD1_OPMODE_IR_CONT; > + break; > + default: > + chip->opmode = ISL29033_CMD1_OPMODE_POWER_DOWN; > + break; > + } > + else > + chip->opmode = ISL29033_CMD1_OPMODE_POWER_DOWN; > + ret = isl29033_set_mode(chip, chip->opmode); > + if (ret != 0) > + chip->opmode = ISL29033_CMD1_OPMODE_POWER_DOWN; > + break; > + default: > + break; > + } > + > +write_done: > + mutex_unlock(&chip->lock); > + > + return ret; > +} > + > +static int isl29033_read_raw(struct iio_dev *indio_dev, > + struct iio_chan_spec const *chan, > + int *val, > + int *val2, > + long mask) > +{ > + int ret = -EINVAL; > + struct isl29033_chip *chip = iio_priv(indio_dev); > + > + mutex_lock(&chip->lock); > + if (chip->suspended) { > + ret = -EBUSY; > + goto read_done; > + } > + switch (mask) { > + case IIO_CHAN_INFO_RAW: > + if (chip->opmode != ISL29033_CMD1_OPMODE_IR_CONT) > + ret = -EBUSY; > + else > + ret = isl29033_read_ir(chip, val); > + break; > + case IIO_CHAN_INFO_PROCESSED: > + switch (chan->type) { > + case IIO_LIGHT: > + if (chip->opmode != ISL29033_CMD1_OPMODE_ALS_CONT) > + ret = -EBUSY; > + else > + ret = isl29033_read_lux(chip, val, val2); > + break; > + case IIO_INTENSITY: > + if (chip->opmode != ISL29033_CMD1_OPMODE_IR_CONT) > + ret = -EBUSY; > + else > + ret = isl29033_read_ir(chip, val); > + break; > + default: > + break; > + } > + break; > + case IIO_CHAN_INFO_INT_TIME: > + if (chan->type == IIO_LIGHT) { > + *val = 0; > + *val2 = isl29033_int_utimes[chip->int_time] > + * chip->rext / ISL29033_REF_REXT; > + ret = IIO_VAL_INT_PLUS_MICRO; > + } > + break; > + case IIO_CHAN_INFO_SCALE: > + if (chan->type == IIO_LIGHT) { > + *val = isl29033_rext_normalize(chip, chip->scale.scale); > + *val2 = isl29033_rext_normalize2(chip, > + chip->scale.scale, chip->scale.uscale); > + ret = IIO_VAL_INT_PLUS_MICRO; > + } > + break; > + case IIO_CHAN_INFO_CALIBSCALE: > + if (chan->type == IIO_LIGHT) { > + *val = chip->calibscale; > + *val2 = chip->ucalibscale; > + ret = IIO_VAL_INT_PLUS_MICRO; > + } > + break; > + case IIO_CHAN_INFO_ENABLE: > + switch (chan->type) { > + case IIO_LIGHT: > + *val = chip->opmode == ISL29033_CMD1_OPMODE_ALS_CONT; > + break; > + case IIO_INTENSITY: > + *val = chip->opmode == ISL29033_CMD1_OPMODE_IR_CONT; > + break; > + default: > + break; > + } > + ret = IIO_VAL_INT; > + break; > + > + default: > + break; > + } > + > +read_done: > + mutex_unlock(&chip->lock); > + > + return ret; > +} > + > +#define ISL29033_LIGHT_CHANNEL { \ > + .type = IIO_LIGHT, \ > + .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) | \ > + BIT(IIO_CHAN_INFO_CALIBSCALE) | \ > + BIT(IIO_CHAN_INFO_SCALE) | \ > + BIT(IIO_CHAN_INFO_INT_TIME) | \ > + BIT(IIO_CHAN_INFO_ENABLE), \ > +} > + > +#define ISL29033_IR_CHANNEL { \ > + .type = IIO_INTENSITY, \ > + .modified = 1, \ > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \ > + BIT(IIO_CHAN_INFO_ENABLE), \ > + .channel2 = IIO_MOD_LIGHT_IR, \ > +} > + > +static const struct iio_chan_spec isl29033_channels[] = { > + ISL29033_LIGHT_CHANNEL, > + ISL29033_IR_CHANNEL, > +}; > + > +static IIO_DEVICE_ATTR(in_illuminance_integration_time_available, 0444, > + isl29033_in_illuminance_integration_time_available, NULL, 0); > +static IIO_DEVICE_ATTR(in_illuminance_scale_available, 0444, > + isl29033_in_illuminance_scale_available, NULL, 0); > + > +#define ISL29033_DEV_ATTR(name) (&iio_dev_attr_##name.dev_attr.attr) > + > +static struct attribute *isl29033_attributes[] = { > + ISL29033_DEV_ATTR(in_illuminance_scale_available), > + ISL29033_DEV_ATTR(in_illuminance_integration_time_available), > + NULL > +}; > + > +static const struct attribute_group isl29033_group = { > + .attrs = isl29033_attributes, > +}; > + > +static int isl29033_chip_init(struct isl29033_chip *chip) > +{ > + int ret; > + struct device *dev = regmap_get_device(chip->regmap); > + > + > + /* > + * Code added per Intersil Application Note 1534: > + * When VDD sinks to approximately 1.8V or below, some of > + * the part's registers may change their state. When VDD > + * recovers to 2.25V (or greater), the part may thus be in an > + * unknown mode of operation. The user can return the part to > + * a known mode of operation either by (a) setting VDD = 0V for > + * 1 second or more and then powering back up with a slew rate > + * of 0.5V/ms or greater, or (b) via I2C disable all ALS/PROX > + * conversions, clear the test registers, and then rewrite all > + * registers to the desired values. > + * ... > + * For ISL29033, etc. > + * 1. Write 0x00 to register 0x08 (TEST) > + * 2. Write 0x00 to register 0x00 (CMD1) > + * 3. Rewrite all registers to the desired values > + */ > + ret = regmap_write(chip->regmap, ISL29033_REG_TEST, 0x0); > + if (ret < 0) { > + dev_err(dev, "Failed to clear isl29033 TEST reg with err %d\n", > + ret); > + return ret; > + } > + > + /* > + * See Intersil AN1534 comments above. > + * "Operating Mode" (COMMAND1) register is reprogrammed when > + * data is read from the device. > + */ > + ret = regmap_write(chip->regmap, ISL29033_REG_ADD_COMMAND1, 0); > + if (ret < 0) { > + dev_err(dev, "Failed to clear isl29033 CMD1 reg with err %d\n", > + ret); > + return ret; > + } > + > + usleep_range(1000, 2000); /* per data sheet, page 10 */ > + > + /* Set defaults */ > + ret = isl29033_set_scale(chip, > + isl29033_rext_normalize(chip, chip->scale.scale), > + isl29033_rext_normalize2(chip, chip->scale.scale, > + chip->scale.uscale)); > + if (ret < 0) { > + dev_err(dev, > + "Init of isl29033 fails (scale) with err %d\n", ret); > + return ret; > + } > + > + ret = isl29033_set_integration_time(chip, > + isl29033_int_utimes[chip->int_time] > + * chip->rext / ISL29033_REF_REXT); > + if (ret < 0) > + dev_err(dev, > + "Init of isl29033 fails (integration) with err %d\n", > + ret); > + > + return ret; > +} > + > +static const struct iio_info isl29033_info = { > + .attrs = &isl29033_group, > + .read_raw = isl29033_read_raw, > + .write_raw = isl29033_write_raw, > +}; > + > +static bool isl29033_is_volatile_reg(struct device *dev, unsigned int reg) > +{ > + switch (reg) { > + case ISL29033_REG_ADD_DATA_LSB: > + case ISL29033_REG_ADD_DATA_MSB: > + case ISL29033_REG_ADD_COMMAND1: > + case ISL29033_REG_ADD_COMMAND2: > + case ISL29033_REG_TEST: > + return true; > + default: > + return false; > + } > +} > + > +static const struct regmap_config isl29033_regmap_config = { > + .reg_bits = 8, > + .val_bits = 8, > + .volatile_reg = isl29033_is_volatile_reg, > + .max_register = ISL29033_REG_TEST, > + .num_reg_defaults_raw = ISL29033_REG_TEST + 1, > + .cache_type = REGCACHE_RBTREE, > +}; > + > +struct isl29033_chip_info { > + const struct iio_chan_spec *channels; > + int num_channels; > + const struct iio_info *info; > + const struct regmap_config *regmap_cfg; > +}; > + > +static const char *isl29033_match_acpi_device(struct device *dev, int *data) > +{ > + const struct acpi_device_id *id; > + > + id = acpi_match_device(dev->driver->acpi_match_table, dev); > + > + if (!id) > + return NULL; > + > + *data = (int)id->driver_data; > + > + return dev_name(dev); > +} > + > +static int isl29033_probe(struct i2c_client *client, > + const struct i2c_device_id *id) > +{ > + struct isl29033_chip *chip; > + struct iio_dev *indio_dev; > + int ret; > + const char *name = NULL; > + int dev_id = 0; > + > + indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*chip)); > + if (!indio_dev) > + return -ENOMEM; > + > + chip = iio_priv(indio_dev); > + > + i2c_set_clientdata(client, indio_dev); > + > + if (id) { > + name = id->name; > + dev_id = id->driver_data; > + } > + > + if (ACPI_HANDLE(&client->dev)) > + name = isl29033_match_acpi_device(&client->dev, &dev_id); > + > + mutex_init(&chip->lock); > + > + chip->calibscale = 1; > + chip->ucalibscale = 0; > + chip->int_time = ISL29033_INT_TIME_16; > + chip->scale = isl29033_scales[chip->int_time][0]; > + chip->suspended = false; > + > +#ifdef CONFIG_OF > + ret = device_property_read_u32(&client->dev, "rext", &chip->rext); T > + if (ret != 0) > + chip->rext = ISL29033_REF_REXT; > +#else > + chip->rext = ISL29033_REF_REXT; > +#endif /* CONFIG_OF */ > + > + chip->regmap = devm_regmap_init_i2c(client, &isl29033_regmap_config); > + > + if (IS_ERR(chip->regmap)) { > + ret = PTR_ERR(chip->regmap); > + dev_err(&client->dev, > + "regmap initialization fails with err %d\n", ret); > + return ret; > + } > + > + ret = isl29033_chip_init(chip); > + if (ret) > + return ret; > + > + indio_dev->info = &isl29033_info; > + indio_dev->channels = isl29033_channels; > + indio_dev->num_channels = ARRAY_SIZE(isl29033_channels); > + indio_dev->name = name; > + indio_dev->dev.parent = &client->dev; > + indio_dev->modes = INDIO_DIRECT_MODE; > + > + return devm_iio_device_register(&client->dev, indio_dev); Given you have a power down mode, it would make sense to do that on device remove. That means you'll probably need a remove function and can't use devm_ version here without breaking ordering. > +} > + > +static int __maybe_unused isl29033_suspend(struct device *dev) > +{ > + struct isl29033_chip *chip = iio_priv(dev_get_drvdata(dev)); > + int ret; > + > + mutex_lock(&chip->lock); > + > + ret = isl29033_set_mode(chip, ISL29033_CMD1_OPMODE_POWER_DOWN); > + if (ret == 0) > + chip->suspended = true; Hmm. I'm not sure we need to be this worried about racing suspend and reads, but it does no harm. > + > + mutex_unlock(&chip->lock); > + > + return ret; > +} > + > +static int __maybe_unused isl29033_resume(struct device *dev) > +{ > + struct isl29033_chip *chip = iio_priv(dev_get_drvdata(dev)); > + int ret; > + > + mutex_lock(&chip->lock); > + > + ret = isl29033_chip_init(chip); > + if (!ret) > + chip->suspended = false; > + > + mutex_unlock(&chip->lock); > + > + return ret; > +} > + > +static const struct dev_pm_ops isl29033_dev_pm_ops = { > + SET_SYSTEM_SLEEP_PM_OPS(isl29033_suspend, isl29033_resume) > +}; > + > +static const struct acpi_device_id isl29033_acpi_match[] = { > + {"ISL29033", 0}, > + {}, > +}; > +MODULE_DEVICE_TABLE(acpi, isl29033_acpi_match); > + > +static const struct i2c_device_id isl29033_id[] = { > + {"isl29033", 0}, > + {} > +}; > +MODULE_DEVICE_TABLE(i2c, isl29033_id); > + > +static const struct of_device_id isl29033_of_match[] = { > + { .compatible = "isil,isl29033", }, > + { }, > +}; > +MODULE_DEVICE_TABLE(of, isl29033_of_match); > + > +static struct i2c_driver isl29033_driver = { > + .driver = { > + .name = "isl29033", > + .acpi_match_table = ACPI_PTR(isl29033_acpi_match), > + .pm = &isl29033_dev_pm_ops, > + .of_match_table = isl29033_of_match, > + }, > + .probe = isl29033_probe, > + .id_table = isl29033_id, > +}; > +module_i2c_driver(isl29033_driver); > + > +MODULE_DESCRIPTION("ISL29033 Ambient Light Sensor driver"); > +MODULE_LICENSE("GPL"); -- To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
