lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date:	Sat, 11 Jun 2016 18:17:54 +0100
From:	Jonathan Cameron <jic23@...nel.org>
To:	Crestez Dan Leonard <leonard.crestez@...el.com>,
	linux-iio@...r.kernel.org
Cc:	linux-kernel@...r.kernel.org, Hartmut Knaack <knaack.h@....de>,
	Lars-Peter Clausen <lars@...afoo.de>,
	Peter Meerwald-Stadler <pmeerw@...erw.net>,
	Daniel Baluta <daniel.baluta@...el.com>
Subject: Re: [PATCH v2] iio: Add driver for Silabs si1132, si1141/2/3 and
 si1145/6/7 ambient light, uv index and proximity sensors

On 30/05/16 20:27, Crestez Dan Leonard wrote:
> From: Peter Meerwald <pmeerw@...erw.net>
> 
> The si114x supports x=1,2,3 IR LEDs for proximity sensing together with
> visible and IR ambient light sensing (ALS).
> 
> Newer parts (si1132, si1145/6/7) can measure UV light and compute an UV index
> 
> Signed-off-by: Peter Meerwald <pmeerw@...erw.net>
> Signed-off-by: Crestez Dan Leonard <leonard.crestez@...el.com>
Various comments inline.

Looking pretty good. Sorry it took me so long to get to it. Been a bit
swamped!

Jonathan
> ---
> 
> Changes since Peter's v1:
> * Fix set_chlist returning positive value on success
> * Do not assume channel addresses are in order of scan index
> * Take lock in buffer_preenable
> * Print part/rev/seq id at probe time
> * Cleanup param query/update
> * Poll for response when executing commands
> * Allow writes even when buffer enabled
> * Add interrupt support
> * Rename new to uncompressed_meas_rate for clarity
> * Fix handling IIO_CHAN_INFO_SCALE:
> Make it so that when changing the scale the processed value remains
> identical in identical conditions. The exposed value should be
> proportional to 2 ** -ADC_GAIN.
> * Initialize UCOEF registers to default values from datasheet
> Otherwise by default the coefficients are zero and so are all the
> values read back
> * Fix reading in_uvindex_raw manually (not in buffer mode)
> * Expose in_uvindex_scale=0.01
> * Expose ADC offset, making zero values meaningful
> This is not very well documented, for example the si1145/6/7 datasheet
> lists register 0x1a as 'reserved'. It might make sense to just return a
> constant on these models.
> 
> This patch is a result of squashing all my changes. I can post this as a
> large expanded series instead but I'm not sure that helps. Interrupt
> support is technically an additional feature but most of the other
> changes are bugfixes. It doesn't make sense to commit a version exposing
> IIO_CHAN_INFO_SCALE incorrectly.
> 
> I tested this on si1143 and si1145
> 
>  drivers/iio/light/Kconfig  |   13 +
>  drivers/iio/light/Makefile |    1 +
>  drivers/iio/light/si1145.c | 1346 ++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 1360 insertions(+)
>  create mode 100644 drivers/iio/light/si1145.c
> 
> diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig
> index 7c566f5..c362e63 100644
> --- a/drivers/iio/light/Kconfig
> +++ b/drivers/iio/light/Kconfig
> @@ -266,6 +266,19 @@ config PA12203001
>           This driver can also be built as a module.  If so, the module
>           will be called pa12203001.
>  
> +config SI1145
> +	tristate "SI1132 and SI1141/2/3/5/6/7 combined ALS, UV index and proximity sensor"
> +	depends on I2C
> +	select IIO_BUFFER
> +	select IIO_TRIGGERED_BUFFER
> +	help
> +	  Say Y here if you want to build a driver for the Silicon Labs SI1132 or
> +	  SI1141/2/3/5/6/7 combined ambient light, UV index and proximity sensor
> +	  chips.
> +
> +	  To compile this driver as a module, choose M here: the module will be
> +	  called si1145.
> +
>  config STK3310
>  	tristate "STK3310 ALS and proximity sensor"
>  	depends on I2C
> diff --git a/drivers/iio/light/Makefile b/drivers/iio/light/Makefile
> index 6f2a3c6..c5768df 100644
> --- a/drivers/iio/light/Makefile
> +++ b/drivers/iio/light/Makefile
> @@ -26,6 +26,7 @@ obj-$(CONFIG_OPT3001)		+= opt3001.o
>  obj-$(CONFIG_PA12203001)	+= pa12203001.o
>  obj-$(CONFIG_RPR0521)		+= rpr0521.o
>  obj-$(CONFIG_SENSORS_TSL2563)	+= tsl2563.o
> +obj-$(CONFIG_SI1145)		+= si1145.o
>  obj-$(CONFIG_STK3310)          += stk3310.o
>  obj-$(CONFIG_TCS3414)		+= tcs3414.o
>  obj-$(CONFIG_TCS3472)		+= tcs3472.o
> diff --git a/drivers/iio/light/si1145.c b/drivers/iio/light/si1145.c
> new file mode 100644
> index 0000000..3b25d40
> --- /dev/null
> +++ b/drivers/iio/light/si1145.c
> @@ -0,0 +1,1346 @@
> +/*
> + * si1145.c - Support for Silabs SI1132 and SI1141/2/3/5/6/7 combined ambient
> + * light, UV index and proximity sensors
> + *
> + * Copyright 2014 Peter Meerwald-Stadler <pmeerw@...erw.net>
Seems like you ought to have some copyright as well Leonard!
> + *
> + * This file is subject to the terms and conditions of version 2 of
> + * the GNU General Public License.  See the file COPYING in the main
> + * directory of this archive for more details.
> + *
> + * SI1132 (7-bit I2C slave address 0x60)
> + * SI1141/2/3 (7-bit I2C slave address 0x5a)
> + * SI1145/6/6 (7-bit I2C slave address 0x60)
> + *
> + * TODO: power management
> + */
> +
> +#include <linux/module.h>
> +#include <linux/i2c.h>
> +#include <linux/err.h>
> +#include <linux/slab.h>
> +#include <linux/delay.h>
> +#include <linux/irq.h>
> +#include <linux/gpio.h>
> +
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +#include <linux/iio/trigger.h>
> +#include <linux/iio/trigger_consumer.h>
> +#include <linux/iio/triggered_buffer.h>
> +#include <linux/iio/buffer.h>
> +#include <linux/util_macros.h>
> +
> +#define SI1145_REG_PART_ID		0x00
> +#define SI1145_REG_REV_ID		0x01
> +#define SI1145_REG_SEQ_ID		0x02
> +#define SI1145_REG_INT_CFG		0x03
> +#define SI1145_REG_IRQ_ENABLE		0x04
> +#define SI1145_REG_IRQ_MODE		0x05
> +#define SI1145_REG_HW_KEY		0x07
> +#define SI1145_REG_MEAS_RATE		0x08
> +#define SI1145_REG_PS_LED21		0x0f
> +#define SI1145_REG_PS_LED3		0x10
> +#define SI1145_REG_UCOEF1		0x13
> +#define SI1145_REG_UCOEF2		0x14
> +#define SI1145_REG_UCOEF3		0x15
> +#define SI1145_REG_UCOEF4		0x16
> +#define SI1145_REG_PARAM_WR		0x17
> +#define SI1145_REG_COMMAND		0x18
> +#define SI1145_REG_RESPONSE		0x20
> +#define SI1145_REG_IRQ_STATUS		0x21
> +#define SI1145_REG_ALSVIS_DATA		0x22
> +#define SI1145_REG_ALSIR_DATA		0x24
> +#define SI1145_REG_PS1_DATA		0x26
> +#define SI1145_REG_PS2_DATA		0x28
> +#define SI1145_REG_PS3_DATA		0x2a
> +#define SI1145_REG_AUX_DATA		0x2c
> +#define SI1145_REG_PARAM_RD		0x2e
> +#define SI1145_REG_CHIP_STAT		0x30
> +
> +#define SI1145_UCOEF1_DEFAULT		0x7b
> +#define SI1145_UCOEF2_DEFAULT		0x6b
> +#define SI1145_UCOEF3_DEFAULT		0x01
> +#define SI1145_UCOEF4_DEFAULT		0x00
> +
> +/* Helper to figure out PS_LED register / shift per channel */
> +#define SI1145_PS_LED_REG(ch) \
> +	(((ch) == 2) ? SI1145_REG_PS_LED3 : SI1145_REG_PS_LED21)
> +#define SI1145_PS_LED_SHIFT(ch) \
> +	(((ch) == 1) ? 4 : 0)
> +
> +/* Parameter offsets */
> +#define SI1145_PARAM_CHLIST		0x01
> +#define SI1145_PARAM_PSLED12_SELECT	0x02
> +#define SI1145_PARAM_PSLED3_SELECT	0x03
> +#define SI1145_PARAM_PS_ENCODING	0x05
> +#define SI1145_PARAM_ALS_ENCODING	0x06
> +#define SI1145_PARAM_PS1_ADC_MUX	0x07
> +#define SI1145_PARAM_PS2_ADC_MUX	0x08
> +#define SI1145_PARAM_PS3_ADC_MUX	0x09
> +#define SI1145_PARAM_PS_ADC_COUNTER	0x0a
> +#define SI1145_PARAM_PS_ADC_GAIN	0x0b
> +#define SI1145_PARAM_PS_ADC_MISC	0x0c
> +#define SI1145_PARAM_ALS_ADC_MUX	0x0d
> +#define SI1145_PARAM_ALSIR_ADC_MUX	0x0e
> +#define SI1145_PARAM_AUX_ADC_MUX	0x0f
> +#define SI1145_PARAM_ALSVIS_ADC_COUNTER	0x10
> +#define SI1145_PARAM_ALSVIS_ADC_GAIN	0x11
> +#define SI1145_PARAM_ALSVIS_ADC_MISC	0x12
> +#define SI1145_PARAM_LED_RECOVERY	0x1c
> +#define SI1145_PARAM_ALSIR_ADC_COUNTER	0x1d
> +#define SI1145_PARAM_ALSIR_ADC_GAIN	0x1e
> +#define SI1145_PARAM_ALSIR_ADC_MISC	0x1f
> +#define SI1145_PARAM_ADC_OFFSET		0x1a
> +
> +/* Channel enable masks for CHLIST parameter */
> +#define SI1145_CHLIST_EN_PS1		0x01
> +#define SI1145_CHLIST_EN_PS2		0x02
> +#define SI1145_CHLIST_EN_PS3		0x04
> +#define SI1145_CHLIST_EN_ALSVIS		0x10
> +#define SI1145_CHLIST_EN_ALSIR		0x20
> +#define SI1145_CHLIST_EN_AUX		0x40
> +#define SI1145_CHLIST_EN_UV		0x80
> +
> +/* Signal range mask for ADC_MISC parameter */
> +#define SI1145_ADC_MISC_RANGE		0x20
> +
> +/* Commands for REG_COMMAND */
> +#define SI1145_CMD_NOP			0x00
> +#define SI1145_CMD_RESET		0x01
> +#define SI1145_CMD_PS_FORCE		0x05
> +#define SI1145_CMD_ALS_FORCE		0x06
> +#define SI1145_CMD_PSALS_FORCE		0x07
> +#define SI1145_CMD_PS_PAUSE		0x09
> +#define SI1145_CMD_ALS_PAUSE		0x0a
> +#define SI1145_CMD_PSALS_PAUSE		0x0b
> +#define SI1145_CMD_PS_AUTO		0x0d
> +#define SI1145_CMD_ALS_AUTO		0x0e
> +#define SI1145_CMD_PSALS_AUTO		0x0f
> +#define SI1145_CMD_PARAM_QUERY		0x80
> +#define SI1145_CMD_PARAM_SET		0xa0
> +
> +#define SI1145_RSP_INVALID_SETTING	0x80
> +#define SI1145_RSP_COUNTER_MASK		0x0F
> +
> +/* Minimum sleep after each command to ensure it's received */
> +#define SI1145_COMMAND_MIN_SLEEP_MS	5
> +/* Return -ETIMEDOUT after this long	*/
> +#define SI1145_COMMAND_TIMEOUT_MS	25
> +
> +/* Interrupt configuration masks for INT_CFG register */
> +#define SI1145_INT_CFG_OE		0x01 /* enable interrupt */
> +#define SI1145_INT_CFG_MODE		0x02 /* auto reset interrupt pin */
> +
> +/* Interrupt enable masks for IRQ_ENABLE register */
> +#define SI1145_BIT_PS3_IE		0x10
> +#define SI1145_BIT_PS2_IE		0x08
> +#define SI1145_BIT_PS1_IE		0x04
> +#define SI1145_BIT_ALS_IE		0x01
> +#define SI1145_MASK_ALL_IE		0x1d
> +
> +#define SI1145_MUX_TEMP			0x65
> +#define SI1145_MUX_VDD			0x75
> +
> +enum {
> +	SI1132,
> +	SI1141,
> +	SI1142,
> +	SI1143,
> +	SI1145,
> +	SI1146,
> +	SI1147,
> +};
> +
> +struct si1145_part_info {
> +	u8 part;
> +	const struct iio_info *iio_info;
> +	const struct iio_chan_spec *channels;
> +	unsigned int num_channels;
> +	unsigned int num_leds;
> +	bool uncompressed_meas_rate;
> +};
> +
> +/**
> + * struct si1145_data - si1145 chip state data
> + * @client:	I2C client
Please document all parameters.
> + * @cmdlock:	mutex to protect command execution only
> + * @rsp_seq:	Next expected response number or -1 if counter reset required
> + * @lock:	mutex to protect shared state.
> + * @part_info:	Part information
> + * @meas_rate:	Value of MEAS_RATE register. Only set in HW in auto mode
> + **/
> +struct si1145_data {
> +	struct i2c_client *client;
> +	struct mutex lock;
> +	struct mutex cmdlock;
> +	int rsp_seq;
> +	const struct si1145_part_info *part_info;
> +	unsigned long scan_mask;
> +	bool autonomous;
> +	struct iio_trigger *trig;
> +	int meas_rate;
> +};
> +
> +/* Send CMD_NOP and wait for response 0
> + *
> + * Returns 0 on success and -errno on error.
> + *
> + * Does not modify data->rsp_seq
> + */
> +static int __si1145_command_reset(struct si1145_data *data)
> +{
> +	struct device *dev = &data->client->dev;
> +	unsigned long start_jiffies = jiffies;
> +	int ret;
> +
> +	ret = i2c_smbus_write_byte_data(data->client, SI1145_REG_COMMAND, SI1145_CMD_NOP);
> +	if (ret < 0)
> +		return ret;
> +	msleep(SI1145_COMMAND_MIN_SLEEP_MS);
> +
> +	while (true) {
> +		ret = i2c_smbus_read_byte_data(data->client, SI1145_REG_RESPONSE);
> +		if (ret < 0)
> +			return ret;
> +		if (ret == 0)
> +			return 0;
> +		if (time_after(jiffies, start_jiffies + SI1145_COMMAND_TIMEOUT_MS * HZ / 1000)) {
> +			dev_warn(dev, "timeout on reset\n");
> +			return -ETIMEDOUT;
> +		}
> +		msleep(5);
> +		continue;
> +	}
> +}
> +
> +/* Execute a command and poll the response register
Please fix up comment syntax throughout...
> + *
> + * Returns 0 on success or -errno on failure
> + *
> + * All conversion overflows are reported as -EOVERFLOW
> + * INVALID_SETTING is reported as -EINVAL
> + * Timeouts are reported as -ETIMEDOUT
> + */
> +static int si1145_command(struct si1145_data *data, u8 cmd)
> +{
> +	struct device *dev = &data->client->dev;
> +	unsigned long start_jiffies = jiffies;
> +	int ret;
> +
> +	mutex_lock(&data->cmdlock);
> +
> +	if (data->rsp_seq < 0) {
> +		ret = __si1145_command_reset(data);
> +		if (ret < 0) {
> +			dev_err(dev, "Failed to reset command counter: %d\n", ret);
> +			goto out;
> +		}
> +		data->rsp_seq = 0;
> +	}
> +
> +	ret = i2c_smbus_write_byte_data(data->client, SI1145_REG_COMMAND, cmd);
> +	if (ret) {
> +		dev_warn(dev, "failed to write command ret=%d\n", ret);
> +		goto out;
> +	}
> +	/* Sleep a little to ensure the command is received */
> +	msleep(SI1145_COMMAND_MIN_SLEEP_MS);
> +
> +	while (true) {
> +		ret = i2c_smbus_read_byte_data(data->client, SI1145_REG_RESPONSE);
> +		if (ret < 0) {
> +			dev_warn(dev, "failed to read response ret=%d\n", ret);
> +			goto out;
> +		}
> +
> +		if ((ret & ~SI1145_RSP_COUNTER_MASK) == 0) {
> +			if (ret == data->rsp_seq) {
> +				if (time_after(jiffies, start_jiffies + SI1145_COMMAND_TIMEOUT_MS * HZ / 1000)) {
> +					dev_warn(dev, "timeout on command %#02hhx\n", cmd);
> +					ret = -ETIMEDOUT;
> +					goto out;
> +				}
> +				msleep(5);
> +				continue;
> +			}
> +			if (ret == ((data->rsp_seq + 1) & SI1145_RSP_COUNTER_MASK)) {
> +				data->rsp_seq = ret;
> +				ret = 0;
> +				goto out;
> +			}
> +			dev_warn(dev, "Unexpected response counter %d instead of %d\n",
> +					ret, (data->rsp_seq + 1) & SI1145_RSP_COUNTER_MASK);
> +			/* Force a counter reset next time */
> +			data->rsp_seq = -1;
> +			ret = -EIO;
> +			goto out;
> +		} else {
> +			if (ret == SI1145_RSP_INVALID_SETTING) {
> +				dev_warn(dev, "INVALID_SETTING error on command %#02hhx\n", cmd);
> +				ret = -EINVAL;
> +			} else {
> +				/* All overflows are treated identically */
> +				dev_dbg(dev, "Overflow ret=%#02hhx cmd=%#02hhx\n", ret, cmd);
> +				ret = -EOVERFLOW;
> +			}
> +			/* Force a counter reset next time */
> +			data->rsp_seq = -1;
> +			goto out;
> +		}
> +	}
> +
> +out:
> +	mutex_unlock(&data->cmdlock);
> +	return ret;
> +}
> +
> +static int si1145_param_update(struct si1145_data *data, u8 op, u8 param, u8 value)
> +{
> +	int ret;
> +
> +	ret = i2c_smbus_write_byte_data(data->client,
> +		SI1145_REG_PARAM_WR, value);
> +	if (ret < 0)
> +		return ret;
> +
> +	return si1145_command(data, op | (param & 0x1F));
> +}
> +
> +static int si1145_param_set(struct si1145_data *data, u8 param, u8 value)
> +{
> +	return si1145_param_update(data, SI1145_CMD_PARAM_SET, param, value);
> +}
> +
> +/* Set param. Returns negative errno or current value */
> +static int si1145_param_query(struct si1145_data *data, u8 param)
> +{
> +	int ret;
> +
> +	ret = si1145_command(data, SI1145_CMD_PARAM_QUERY | (param & 0x1F));
> +	if (ret < 0)
> +		return ret;
> +
> +	return i2c_smbus_read_byte_data(data->client, SI1145_REG_PARAM_RD);
> +}
> +
> +/* expand 8 bit compressed value to 16 bit, see Silabs AN498 */
> +static u16 si1145_uncompress(u8 x)
> +{
> +	u16 result = 0;
> +	u8 exponent = 0;
> +
> +	if (x < 8)
> +		return 0;
> +
> +	exponent = (x & 0xf0) >> 4;
> +	result = 0x10 | (x & 0x0f);
> +
> +	if (exponent >= 4)
> +		return result << (exponent - 4);
> +	return result >> (4 - exponent);
> +}
> +
> +/* compress 16 bit to 8 bit using 4 bit exponent and 4 bit fraction,
> + * see Silabs AN498 */
> +static u8 si1145_compress(u16 x)
> +{
> +	u32 exponent = 0;
> +	u32 significand = 0;
> +	u32 tmp = x;
> +
> +	if (x == 0x0000)
> +		return 0x00;
> +	if (x == 0x0001)
> +		return 0x08;
> +
> +	while (1) {
> +		tmp >>= 1;
> +		exponent += 1;
> +		if (tmp == 1)
> +			break;
> +	}
> +
> +	if (exponent < 5) {
> +		significand = x << (4 - exponent);
> +		return (exponent << 4) | (significand & 0xF);
> +	}
> +
> +	significand = x >> (exponent - 5);
> +	if (significand & 1) {
> +		significand += 2;
> +		if (significand & 0x0040) {
> +			exponent += 1;
> +			significand >>= 1;
> +		}
> +	}
> +
> +	return (exponent << 4) | ((significand >> 1) & 0xF);
> +}
> +
> +/* Write meas_rate in hardware */
> +static int si1145_set_meas_rate(struct si1145_data *data, int interval)
> +{
> +	if (data->part_info->uncompressed_meas_rate)
> +		return i2c_smbus_write_word_data(data->client,
> +			SI1145_REG_MEAS_RATE, interval);
> +	else
> +		return i2c_smbus_write_byte_data(data->client,
> +			SI1145_REG_MEAS_RATE, interval);
> +}
> +
> +static int si1145_read_samp_freq(struct si1145_data *data, int *val, int *val2)
> +{
> +	*val = 32000;
> +	if (data->part_info->uncompressed_meas_rate)
> +		*val2 = data->meas_rate;
> +	else
> +		*val2 = si1145_uncompress(data->meas_rate);
> +	return IIO_VAL_FRACTIONAL;
> +}
> +
> +/* Set the samp freq in driver private data */
> +static int si1145_store_samp_freq(struct si1145_data *data, int val)
> +{
> +	int ret = 0;
> +	int meas_rate;
> +
> +	if (val <= 0 || val > 32000)
> +		return -ERANGE;
> +        meas_rate = 32000 / val;
> +
> +	mutex_lock(&data->lock);
> +	if (data->autonomous) {
> +		ret = si1145_set_meas_rate(data, meas_rate);
> +		if (ret)
> +			goto out;
> +	}
> +	if (data->part_info->uncompressed_meas_rate)
> +		data->meas_rate = meas_rate;
> +	else
> +		data->meas_rate = si1145_compress(meas_rate);
> +
> +out:
> +	mutex_unlock(&data->lock);
> +	return ret;
> +}
> +
> +static irqreturn_t si1145_trigger_handler(int irq, void *private)
> +{
> +	struct iio_poll_func *pf = private;
> +	struct iio_dev *indio_dev = pf->indio_dev;
> +	struct si1145_data *data = iio_priv(indio_dev);
> +	/*
> +	 * Maximum buffer size:
> +	 *   6*2 bytes channels data + 4 bytes alignment +
> +	 *   8 bytes timestamp
> +	 */
Nice comment.  If every driver put it like this we'd get far fewer
bugs around the size of buffer.
> +	u8 buffer[24];
> +	int i, j = 0;
> +	int ret;
> +	u8 irq_status = 0;
> +
> +	if (!data->autonomous) {
> +		ret = si1145_command(data, SI1145_CMD_PSALS_FORCE);
> +		if (ret < 0 && ret != -EOVERFLOW)
> +			goto done;
> +	} else {
> +		irq_status = ret = i2c_smbus_read_byte_data(data->client,
> +				SI1145_REG_IRQ_STATUS);
> +		if (ret < 0)
> +			goto done;
> +		if (!(irq_status & SI1145_MASK_ALL_IE))
> +			goto done;
> +	}
> +
> +	for_each_set_bit(i, indio_dev->active_scan_mask,
> +		indio_dev->masklength) {
> +		int run = 1;
> +
> +		while (i + run < indio_dev->masklength) {
> +			if (!test_bit(i + run, indio_dev->active_scan_mask))
> +				break;
> +			if (indio_dev->channels[i + run].address !=
> +					indio_dev->channels[i].address + 2 * run)
> +				break;
> +			run++;
> +		}
> +
> +		if (run > 1) {
> +			ret = i2c_smbus_read_i2c_block_data(data->client,
> +				indio_dev->channels[i].address,
> +				sizeof(u16) * run, &buffer[j]);
> +		} else {
> +			ret = i2c_smbus_read_word_data(data->client,
> +				indio_dev->channels[i].address);
> +			*(u16 *) &buffer[j] = ret;
Hmm. Is there an endianness issue here... Block data
doesn't care, wherease _word data will convert to cpu endian.
Also, would it matter if we just dropped the special case?
If it is more complex than that, please add some explanatory comments.

> +		}
> +		if (ret < 0)
> +			goto done;
> +		j += run * sizeof(u16);
> +		i += run - 1;
> +	}
> +
> +	if (data->autonomous) {
> +		ret = i2c_smbus_write_byte_data(data->client,
> +				SI1145_REG_IRQ_STATUS,
> +				irq_status & SI1145_MASK_ALL_IE);
> +		if (ret < 0)
> +			goto done;
> +	}
> +
> +	iio_push_to_buffers_with_timestamp(indio_dev, buffer,
> +		iio_get_time_ns());
> +
> +done:
> +	iio_trigger_notify_done(indio_dev->trig);
> +	return IRQ_HANDLED;
> +}
> +
> +static int si1145_set_chlist(struct iio_dev *indio_dev, unsigned long scan_mask)
> +{
> +	struct si1145_data *data = iio_priv(indio_dev);
> +	u8 reg = 0, mux;
> +	int ret;
> +	int i;
> +
> +	/* channel list already set, no need to reprogram */
> +	if (data->scan_mask == scan_mask)
> +		return 0;
> +
> +	for_each_set_bit(i, &scan_mask, indio_dev->masklength) {
> +		switch (indio_dev->channels[i].address) {
> +		case SI1145_REG_ALSVIS_DATA:
> +			reg |= SI1145_CHLIST_EN_ALSVIS;
> +			break;
> +		case SI1145_REG_ALSIR_DATA:
> +			reg |= SI1145_CHLIST_EN_ALSIR;
> +			break;
> +		case SI1145_REG_PS1_DATA:
> +			reg |= SI1145_CHLIST_EN_PS1;
> +			break;
> +		case SI1145_REG_PS2_DATA:
> +			reg |= SI1145_CHLIST_EN_PS2;
> +			break;
> +		case SI1145_REG_PS3_DATA:
> +			reg |= SI1145_CHLIST_EN_PS3;
> +			break;
> +		case SI1145_REG_AUX_DATA:
> +			switch (indio_dev->channels[i].type) {
> +			case IIO_UVINDEX:
> +				reg |= SI1145_CHLIST_EN_UV;
> +				break;
> +			default:
> +				reg |= SI1145_CHLIST_EN_AUX;
> +				if (indio_dev->channels[i].type == IIO_TEMP)
> +					mux = SI1145_MUX_TEMP;
> +				else
> +					mux = SI1145_MUX_VDD;
> +				ret = si1145_param_set(data,
> +					SI1145_PARAM_AUX_ADC_MUX, mux);
> +				if (ret < 0)
> +					return ret;
> +
> +				break;
> +			}
> +		}
> +	}
> +
> +	data->scan_mask = scan_mask;
> +	ret = si1145_param_set(data, SI1145_PARAM_CHLIST, reg);
> +
> +	return ret < 0 ? ret : 0;
> +}
> +
> +static int si1145_measure(struct iio_dev *indio_dev,
> +			  struct iio_chan_spec const *chan)
> +{
> +	struct si1145_data *data = iio_priv(indio_dev);
> +	u8 cmd;
> +	int ret;
> +
> +	ret = si1145_set_chlist(indio_dev, BIT(chan->scan_index));
> +	if (ret < 0)
> +		return ret;
> +
> +	cmd = (chan->type == IIO_PROXIMITY) ? SI1145_CMD_PS_FORCE :
> +		SI1145_CMD_ALS_FORCE;
> +	ret = si1145_command(data, cmd);
> +	if (ret < 0 && ret != -EOVERFLOW)
> +		return ret;
> +
> +	return i2c_smbus_read_word_data(data->client, chan->address);
> +}
> +
> +/*
> + * Conversion between iio scale and ADC_GAIN values
> + * These could be further adjusted but proximity/intensity are dimensionless
> + */
> +static const int si1145_proximity_scale_available[] =	{128,64,32,16,8,4};
> +static const int si1145_intensity_scale_available[] =	{128,64,32,16,8,4,2,1};
> +static IIO_CONST_ATTR(in_proximity_scale_available, 	"128 64 32 16 8 4");
> +static IIO_CONST_ATTR(in_intensity_scale_available, 	"128 64 32 16 8 4 2 1");
> +static IIO_CONST_ATTR(in_intensity_ir_scale_available, 	"128 64 32 16 8 4 2 1");
> +
> +static int si1145_scale_from_adcgain(int regval)
> +{
> +	return 128 >> regval;
> +}
> +
> +static int si1145_proximity_adcgain_from_scale(int val, int val2)
> +{
> +	val = find_closest_descending(val, si1145_proximity_scale_available,
> +				ARRAY_SIZE(si1145_proximity_scale_available));
> +	if (val < 0 || val > 5 || val2 != 0)
> +		return -EINVAL;
> +
> +	return val;
> +}
> +
> +static int si1145_intensity_adcgain_from_scale(int val, int val2)
> +{
> +	val = find_closest_descending(val, si1145_intensity_scale_available,
> +				ARRAY_SIZE(si1145_intensity_scale_available));
> +	if (val < 0 || val > 7 || val2 != 0)
> +		return -EINVAL;
> +
> +	return val;
> +}
> +
> +static int si1145_read_raw(struct iio_dev *indio_dev,
> +				struct iio_chan_spec const *chan,
> +				int *val, int *val2, long mask)
> +{
> +	struct si1145_data *data = iio_priv(indio_dev);
> +	int ret;
> +	u8 reg;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		switch (chan->type) {
> +		case IIO_INTENSITY:
> +		case IIO_PROXIMITY:
> +		case IIO_VOLTAGE:
> +		case IIO_TEMP:
> +		case IIO_UVINDEX:
> +			if (iio_buffer_enabled(indio_dev))
> +				return -EBUSY;
> +
> +			mutex_lock(&data->lock);
> +			ret = si1145_measure(indio_dev, chan);
> +			mutex_unlock(&data->lock);
> +			if (ret < 0)
> +				return ret;
> +
> +			*val = ret;
> +
> +			return IIO_VAL_INT;
> +		case IIO_CURRENT:
> +			ret = i2c_smbus_read_byte_data(data->client,
> +				SI1145_PS_LED_REG(chan->channel));
> +			if (ret < 0)
> +				return ret;
> +
> +			*val = (ret >> SI1145_PS_LED_SHIFT(chan->channel))
> +				& 0x0f;
> +
> +			return IIO_VAL_INT;
> +		default:
> +			return -EINVAL;
> +		}
> +	case IIO_CHAN_INFO_SCALE:
> +		switch (chan->type) {
> +		case IIO_PROXIMITY:
> +			reg = SI1145_PARAM_PS_ADC_GAIN;
> +			break;
> +		case IIO_INTENSITY:
> +			if (chan->channel2 == IIO_MOD_LIGHT_IR)
> +				reg = SI1145_PARAM_ALSIR_ADC_GAIN;
> +			else
> +				reg = SI1145_PARAM_ALSVIS_ADC_GAIN;
> +			break;
> +		case IIO_TEMP:
> +			*val = 28;
> +			*val2 = 571429;
> +			return IIO_VAL_INT_PLUS_MICRO;
> +		case IIO_UVINDEX:
> +			*val = 0;
> +			*val2 = 10000;
> +			return IIO_VAL_INT_PLUS_MICRO;
> +		default:
> +			return -EINVAL;
> +		}
> +
> +		ret = si1145_param_query(data, reg);
> +		if (ret < 0)
> +			return ret;
> +
> +		*val = si1145_scale_from_adcgain(ret & 0x07);
> +
> +		return IIO_VAL_INT;
> +	case IIO_CHAN_INFO_OFFSET:
> +		switch (chan->type) {
> +		case IIO_TEMP:
> +			/* -ADC offset - ADC counts @ 25°C - 35 * ADC counts / °C */
> +			*val = -256 - 11136 + 25 * 35;
> +			return IIO_VAL_INT;
> +		default:
> +			/*
> +			 * All ADC measurements have are by default offset by -256
> +			 * See AN498 5.6.3
> +			 */
> +			ret = si1145_param_query(data, SI1145_PARAM_ADC_OFFSET);
> +			if (ret < 0)
> +				return ret;
> +			*val = -si1145_uncompress(ret);
> +			return IIO_VAL_INT;
> +		}
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +		return si1145_read_samp_freq(data, val, val2);
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static int si1145_write_raw(struct iio_dev *indio_dev,
> +			       struct iio_chan_spec const *chan,
> +			       int val, int val2, long mask)
> +{
> +	struct si1145_data *data = iio_priv(indio_dev);
> +	u8 reg1, reg2, shift;
> +	int ret;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_SCALE:
> +		switch (chan->type) {
> +		case IIO_PROXIMITY:
> +			val = si1145_proximity_adcgain_from_scale(val, val2);
> +			if (val < 0)
> +				return val;
> +			reg1 = SI1145_PARAM_PS_ADC_GAIN;
> +			reg2 = SI1145_PARAM_PS_ADC_COUNTER;
> +			break;
> +		case IIO_INTENSITY:
> +			val = si1145_intensity_adcgain_from_scale(val, val2);
> +			if (val < 0)
> +				return val;
> +			if (chan->channel2 == IIO_MOD_LIGHT_IR) {
> +				reg1 = SI1145_PARAM_ALSIR_ADC_GAIN;
> +				reg2 = SI1145_PARAM_ALSIR_ADC_COUNTER;
> +			} else {
> +				reg1 = SI1145_PARAM_ALSVIS_ADC_GAIN;
> +				reg2 = SI1145_PARAM_ALSVIS_ADC_COUNTER;
> +			}
> +			break;
> +		default:
> +			return -EINVAL;
> +		}
> +
> +		ret = si1145_param_set(data, reg1, val);
> +		if (ret < 0)
> +			return ret;
> +		/* Set recovery period to one's complement of gain */
> +		return si1145_param_set(data, reg2, (~val & 0x07) << 4);
> +	case IIO_CHAN_INFO_RAW:
> +		if (chan->type != IIO_CURRENT)
> +			return -EINVAL;
> +
> +		if (val < 0 || val > 15 || val2 != 0)
> +			return -EINVAL;
> +
> +		reg1 = SI1145_PS_LED_REG(chan->channel);
> +		shift = SI1145_PS_LED_SHIFT(chan->channel);
> +		ret = i2c_smbus_read_byte_data(data->client, reg1);
> +		if (ret < 0)
> +			return ret;
> +		return i2c_smbus_write_byte_data(data->client, reg1,
> +			(ret & ~(0x0f << shift)) |
> +			((val & 0x0f) << shift));
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +		return si1145_store_samp_freq(data, val);
> +	}
> +	return -EINVAL;
> +}
> +
> +#define SI1145_ST { \
> +	.sign = 'u', \
> +	.realbits = 16, \
> +	.storagebits = 16, \
> +	.endianness = IIO_LE, \
> +}
> +
> +#define SI1145_INTENSITY_CHANNEL(_si) { \
> +	.type = IIO_INTENSITY, \
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
> +			      BIT(IIO_CHAN_INFO_OFFSET) | \
> +			      BIT(IIO_CHAN_INFO_SCALE), \
> +	.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ), \
> +	.scan_type = SI1145_ST, \
> +	.scan_index = _si, \
> +	.address = SI1145_REG_ALSVIS_DATA, \
> +}
> +
> +#define SI1145_INTENSITY_IR_CHANNEL(_si) { \
> +	.type = IIO_INTENSITY, \
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
> +			      BIT(IIO_CHAN_INFO_OFFSET) | \
> +			      BIT(IIO_CHAN_INFO_SCALE), \
> +	.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ), \
> +	.modified = 1, \
> +	.channel2 = IIO_MOD_LIGHT_IR, \
> +	.scan_type = SI1145_ST, \
> +	.scan_index = _si, \
> +	.address = SI1145_REG_ALSIR_DATA, \
> +}
> +
> +#define SI1145_TEMP_CHANNEL(_si) { \
> +	.type = IIO_TEMP, \
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
> +			      BIT(IIO_CHAN_INFO_OFFSET) | \
> +			      BIT(IIO_CHAN_INFO_SCALE), \
> +	.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ), \
> +	.scan_type = SI1145_ST, \
> +	.scan_index = _si, \
> +	.address = SI1145_REG_AUX_DATA, \
> +}
> +
> +#define SI1145_UV_CHANNEL(_si) { \
> +	.type = IIO_UVINDEX, \
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
> +			      BIT(IIO_CHAN_INFO_SCALE), \
> +	.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ), \
> +	.scan_type = SI1145_ST, \
> +	.scan_index = _si, \
> +	.address = SI1145_REG_AUX_DATA, \
> +}
> +
> +#define SI1145_PROXIMITY_CHANNEL(_si, _ch) { \
> +	.type = IIO_PROXIMITY, \
> +	.indexed = 1, \
> +	.channel = _ch, \
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
> +	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) | \
> +				    BIT(IIO_CHAN_INFO_OFFSET), \
> +	.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ), \
> +	.scan_type = SI1145_ST, \
> +	.scan_index = _si, \
> +	.address = SI1145_REG_PS1_DATA + _ch * 2, \
> +}
> +
> +#define SI1145_VOLTAGE_CHANNEL(_si) { \
> +	.type = IIO_VOLTAGE, \
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
> +	.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ), \
> +	.scan_type = SI1145_ST, \
> +	.scan_index = _si, \
> +	.address = SI1145_REG_AUX_DATA, \
> +}
> +
> +#define SI1145_CURRENT_CHANNEL(_ch) { \
> +	.type = IIO_CURRENT, \
> +	.indexed = 1, \
> +	.channel = _ch, \
> +	.output = 1, \
> +	.scan_index = -1, \
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
> +}
> +
> +static const struct iio_chan_spec si1132_channels[] = {
> +	SI1145_INTENSITY_CHANNEL(0),
> +	SI1145_INTENSITY_IR_CHANNEL(1),
> +	SI1145_TEMP_CHANNEL(2),
> +	SI1145_VOLTAGE_CHANNEL(3),
> +	SI1145_UV_CHANNEL(4),
> +	IIO_CHAN_SOFT_TIMESTAMP(6),
> +};
> +
> +static const struct iio_chan_spec si1141_channels[] = {
> +	SI1145_INTENSITY_CHANNEL(0),
> +	SI1145_INTENSITY_IR_CHANNEL(1),
> +	SI1145_PROXIMITY_CHANNEL(2, 0),
> +	SI1145_TEMP_CHANNEL(3),
> +	SI1145_VOLTAGE_CHANNEL(4),
> +	IIO_CHAN_SOFT_TIMESTAMP(5),
> +	SI1145_CURRENT_CHANNEL(0),
> +};
> +
> +static const struct iio_chan_spec si1142_channels[] = {
> +	SI1145_INTENSITY_CHANNEL(0),
> +	SI1145_INTENSITY_IR_CHANNEL(1),
> +	SI1145_PROXIMITY_CHANNEL(2, 0),
> +	SI1145_PROXIMITY_CHANNEL(3, 1),
> +	SI1145_TEMP_CHANNEL(4),
> +	SI1145_VOLTAGE_CHANNEL(5),
> +	IIO_CHAN_SOFT_TIMESTAMP(6),
> +	SI1145_CURRENT_CHANNEL(0),
> +	SI1145_CURRENT_CHANNEL(1),
> +};
> +
> +static const struct iio_chan_spec si1143_channels[] = {
> +	SI1145_INTENSITY_CHANNEL(0),
> +	SI1145_INTENSITY_IR_CHANNEL(1),
> +	SI1145_PROXIMITY_CHANNEL(2, 0),
> +	SI1145_PROXIMITY_CHANNEL(3, 1),
> +	SI1145_PROXIMITY_CHANNEL(4, 2),
> +	SI1145_TEMP_CHANNEL(5),
> +	SI1145_VOLTAGE_CHANNEL(6),
> +	IIO_CHAN_SOFT_TIMESTAMP(7),
> +	SI1145_CURRENT_CHANNEL(0),
> +	SI1145_CURRENT_CHANNEL(1),
> +	SI1145_CURRENT_CHANNEL(2),
> +};
> +
> +static const struct iio_chan_spec si1145_channels[] = {
> +	SI1145_INTENSITY_CHANNEL(0),
> +	SI1145_INTENSITY_IR_CHANNEL(1),
> +	SI1145_PROXIMITY_CHANNEL(2, 0),
> +	SI1145_TEMP_CHANNEL(3),
> +	SI1145_VOLTAGE_CHANNEL(4),
> +	SI1145_UV_CHANNEL(5),
> +	IIO_CHAN_SOFT_TIMESTAMP(6),
> +	SI1145_CURRENT_CHANNEL(0),
> +};
> +
> +static const struct iio_chan_spec si1146_channels[] = {
> +	SI1145_INTENSITY_CHANNEL(0),
> +	SI1145_INTENSITY_IR_CHANNEL(1),
> +	SI1145_TEMP_CHANNEL(2),
> +	SI1145_VOLTAGE_CHANNEL(3),
> +	SI1145_UV_CHANNEL(4),
> +	SI1145_PROXIMITY_CHANNEL(5, 0),
> +	SI1145_PROXIMITY_CHANNEL(6, 1),
> +	IIO_CHAN_SOFT_TIMESTAMP(7),
> +	SI1145_CURRENT_CHANNEL(0),
> +	SI1145_CURRENT_CHANNEL(1),
> +};
> +
> +static const struct iio_chan_spec si1147_channels[] = {
> +	SI1145_INTENSITY_CHANNEL(0),
> +	SI1145_INTENSITY_IR_CHANNEL(1),
> +	SI1145_PROXIMITY_CHANNEL(2, 0),
> +	SI1145_PROXIMITY_CHANNEL(3, 1),
> +	SI1145_PROXIMITY_CHANNEL(4, 2),
> +	SI1145_TEMP_CHANNEL(5),
> +	SI1145_VOLTAGE_CHANNEL(6),
> +	SI1145_UV_CHANNEL(7),
> +	IIO_CHAN_SOFT_TIMESTAMP(8),
> +	SI1145_CURRENT_CHANNEL(0),
> +	SI1145_CURRENT_CHANNEL(1),
> +	SI1145_CURRENT_CHANNEL(2),
> +};
> +
> +static struct attribute *si1132_attributes[] = {
> +	&iio_const_attr_in_intensity_scale_available.dev_attr.attr,
> +	&iio_const_attr_in_intensity_ir_scale_available.dev_attr.attr,
> +	NULL,
> +};
> +
> +static struct attribute *si114x_attributes[] = {
> +	&iio_const_attr_in_intensity_scale_available.dev_attr.attr,
> +	&iio_const_attr_in_intensity_ir_scale_available.dev_attr.attr,
> +	&iio_const_attr_in_proximity_scale_available.dev_attr.attr,
> +	NULL,
> +};
> +
> +static const struct attribute_group si1132_attribute_group = {
> +	.attrs = si1132_attributes,
> +};
> +
> +static const struct attribute_group si114x_attribute_group = {
> +	.attrs = si114x_attributes,
> +};
> +
> +
> +static const struct iio_info si1132_info = {
> +	.read_raw = si1145_read_raw,
> +	.write_raw = si1145_write_raw,
> +	.driver_module = THIS_MODULE,
> +	.attrs = &si1132_attribute_group,
> +};
> +
> +static const struct iio_info si114x_info = {
> +	.read_raw = si1145_read_raw,
> +	.write_raw = si1145_write_raw,
> +	.driver_module = THIS_MODULE,
> +	.attrs = &si114x_attribute_group,
> +};
> +
> +#define SI1145_PART(id, iio_info, chans, leds, uncompressed_meas_rate) \
> +	{id, iio_info, chans, ARRAY_SIZE(chans), leds, uncompressed_meas_rate}
> +
> +static const struct si1145_part_info si1145_part_info[] = {
> +	[SI1132] = SI1145_PART(0x32, &si1132_info, si1132_channels, 0, true),
> +	[SI1141] = SI1145_PART(0x41, &si114x_info, si1141_channels, 1, false),
> +	[SI1142] = SI1145_PART(0x42, &si114x_info, si1142_channels, 2, false),
> +	[SI1143] = SI1145_PART(0x43, &si114x_info, si1143_channels, 3, false),
> +	[SI1145] = SI1145_PART(0x45, &si114x_info, si1145_channels, 1, true),
> +	[SI1146] = SI1145_PART(0x46, &si114x_info, si1146_channels, 2, true),
> +	[SI1147] = SI1145_PART(0x47, &si114x_info, si1147_channels, 3, true),
> +};
> +
> +static int si1145_initialize(struct si1145_data *data)
> +{
> +	struct i2c_client *client = data->client;
> +	int ret;
> +
> +	ret = i2c_smbus_write_byte_data(client, SI1145_REG_COMMAND,
> +		SI1145_CMD_RESET);
> +	if (ret < 0)
> +		return ret;
> +	msleep(20);
> +
> +	/* Hardware key, magic value */
> +	ret = i2c_smbus_write_byte_data(client, SI1145_REG_HW_KEY, 0x17);
> +	if (ret < 0)
> +		return ret;
> +	msleep(20);
> +
> +	/* Turn off autonomous mode */
> +	ret = si1145_set_meas_rate(data, 0);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* Initialize sampling freq to 10hz */
> +	ret = si1145_store_samp_freq(data, 10);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* Set LED currents to 45 mA */
> +	switch (data->part_info->num_leds) {
> +	case 1:
> +		ret = i2c_smbus_write_byte_data(client,
> +			SI1145_REG_PS_LED21, 0x03);
> +		break;
> +	case 2:
> +		ret = i2c_smbus_write_byte_data(client,
> +			SI1145_REG_PS_LED21, 0x43);
> +		break;
> +	case 3:
> +		ret = i2c_smbus_write_byte_data(client,
> +			SI1145_REG_PS_LED3, 0x03);
> +		if (ret < 0)
> +			return ret;
> +		ret = i2c_smbus_write_byte_data(client,
> +			SI1145_REG_PS_LED21, 0x43);
> +		break;
> +	default:
> +		ret = 0;
> +		break;
> +	}
> +	if (ret < 0)
> +		return ret;
> +
> +	/* Set normal proximity measurement mode */
> +	ret = si1145_param_set(data, SI1145_PARAM_PS_ADC_MISC, 0x04);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = si1145_param_set(data, SI1145_PARAM_PS_ADC_GAIN, 0x01);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = si1145_param_set(data, SI1145_PARAM_PS_ADC_COUNTER, 0x03 << 4);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* Set ALS visible measurement mode */
> +	ret = si1145_param_set(data, SI1145_PARAM_ALSVIS_ADC_MISC, SI1145_ADC_MISC_RANGE);
Line looks a bit long...
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = si1145_param_set(data, SI1145_PARAM_ALSVIS_ADC_GAIN, 0x03);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = si1145_param_set(data, SI1145_PARAM_ALSVIS_ADC_COUNTER, 0x04 << 4);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* Set ALS IR measurement mode */
> +	ret = si1145_param_set(data, SI1145_PARAM_ALSIR_ADC_MISC, SI1145_ADC_MISC_RANGE);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = si1145_param_set(data, SI1145_PARAM_ALSIR_ADC_GAIN, 0x01);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = si1145_param_set(data, SI1145_PARAM_ALSIR_ADC_COUNTER, 0x00 << 4);
> +	if (ret < 0)
> +		return ret;
> +
> +	/*
> +	 * Initialize UCOEF to default values in datasheet
> +	 * These registers are normally zero on reset
> +	 */
> +	if (data->part_info->part == 0x32 ||
> +		data->part_info->part == 0x45 ||
> +		data->part_info->part == 0x46 ||
> +		data->part_info->part == 0x47)
> +	{
> +		ret = i2c_smbus_write_byte_data(data->client,
> +				SI1145_REG_UCOEF1, SI1145_UCOEF1_DEFAULT);
> +		if (ret < 0)
> +			return ret;
> +		ret = i2c_smbus_write_byte_data(data->client,
> +				SI1145_REG_UCOEF2, SI1145_UCOEF2_DEFAULT);
> +		if (ret < 0)
> +			return ret;
> +		ret = i2c_smbus_write_byte_data(data->client,
> +				SI1145_REG_UCOEF3, SI1145_UCOEF3_DEFAULT);
> +		if (ret < 0)
> +			return ret;
> +		ret = i2c_smbus_write_byte_data(data->client,
> +				SI1145_REG_UCOEF4, SI1145_UCOEF4_DEFAULT);
> +		if (ret < 0)
> +			return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static int si1145_buffer_preenable(struct iio_dev *indio_dev)
> +{
> +	struct si1145_data *data = iio_priv(indio_dev);
> +	int ret;
> +
> +	mutex_lock(&data->lock);
> +	ret = si1145_set_chlist(indio_dev, *indio_dev->active_scan_mask);
> +	mutex_unlock(&data->lock);
> +
> +	return ret;
> +}
> +
> +bool si1145_validate_scan_mask(struct iio_dev *indio_dev,
> +			       const unsigned long *scan_mask)
> +{
> +	struct si1145_data *data = iio_priv(indio_dev);
> +	unsigned int count = 0;
> +	int i;
> +
> +	/* Check that at most one AUX channel is enabled */
> +	for_each_set_bit(i, scan_mask, data->part_info->num_channels) {
> +		if (indio_dev->channels[i].address == SI1145_REG_AUX_DATA)
> +			count++;
> +	}
> +
> +	return count <= 1;
> +}
> +
> +static const struct iio_buffer_setup_ops si1145_buffer_setup_ops = {
> +	.preenable = si1145_buffer_preenable,
> +	.postenable = iio_triggered_buffer_postenable,
> +	.predisable = iio_triggered_buffer_predisable,
> +	.validate_scan_mask = si1145_validate_scan_mask,
> +};
> +
> +/* Set trigger state
Fix the multiline comment style please.
> + *
> + * When not using triggers interrupts are disabled and measurement rate is
> + * set to zero in order to minimize power consumption.
> + */
> +static int si1145_trigger_set_state(struct iio_trigger *trig, bool state)
> +{
> +	struct iio_dev *indio_dev = iio_trigger_get_drvdata(trig);
> +	struct si1145_data *data = iio_priv(indio_dev);
> +	int err = 0, ret;
> +
> +	mutex_lock(&data->lock);
> +
> +	if (state) {
> +		data->autonomous = true;
> +		err = i2c_smbus_write_byte_data(data->client,
> +				SI1145_REG_INT_CFG, SI1145_INT_CFG_OE);
> +		if (err < 0)
> +			goto disable;
> +		err = i2c_smbus_write_byte_data(data->client,
> +				SI1145_REG_IRQ_ENABLE, SI1145_MASK_ALL_IE);
> +		if (err < 0)
> +			goto disable;
> +		err = si1145_set_meas_rate(data, data->meas_rate);
> +		if (err < 0)
> +			goto disable;
> +		err = si1145_command(data, SI1145_CMD_PSALS_AUTO);
> +		if (err < 0)
> +			goto disable;
> +	} else {
> +disable:
> +		/* Disable as much as possible skipping errors */
> +		ret = si1145_command(data, state ? SI1145_CMD_PSALS_AUTO : SI1145_CMD_PSALS_PAUSE);
> +		if (ret < 0 && !err)
> +			err = ret;
> +		ret = si1145_set_meas_rate(data, 0);
> +		if (ret < 0 && !err)
> +			err = ret;
> +		ret = i2c_smbus_write_byte_data(data->client, SI1145_REG_IRQ_ENABLE, 0);
> +		if (ret < 0 && !err)
> +			err = ret;
> +		ret = i2c_smbus_write_byte_data(data->client, SI1145_REG_INT_CFG, 0);
> +		if (ret < 0 && !err)
> +			err = ret;
> +		data->autonomous = false;
> +	}
> +
> +	mutex_unlock(&data->lock);
> +	return err;
> +}
> +
> +static const struct iio_trigger_ops si1145_trigger_ops = {
> +	.owner = THIS_MODULE,
> +	.set_trigger_state = si1145_trigger_set_state,
> +};
> +
> +static int si1145_probe_trigger(struct iio_dev *indio_dev)
> +{
> +	struct si1145_data *data = iio_priv(indio_dev);
> +	int ret;
> +
> +	data->trig = devm_iio_trigger_alloc(&data->client->dev,
> +			"%s-dev%d", indio_dev->name, indio_dev->id);
> +	if (!data->trig)
> +		return -ENOMEM;
> +
> +	data->trig->dev.parent = &data->client->dev;
> +	data->trig->ops = &si1145_trigger_ops;
> +	iio_trigger_set_drvdata(data->trig, indio_dev);
> +	ret = iio_trigger_register(data->trig);
> +	if (ret)
> +		return ret;
> +
> +	/* select default trigger */
> +	indio_dev->trig = data->trig;
> +
> +	return 0;
> +}
> +
> +static void si1145_remove_trigger(struct iio_dev *indio_dev)
> +{
> +	struct si1145_data *data = iio_priv(indio_dev);
> +
> +	if (data->trig)
> +		iio_trigger_unregister(data->trig);
> +}
> +
> +static int si1145_probe(struct i2c_client *client,
> +			const struct i2c_device_id *id)
> +{
> +	struct si1145_data *data;
> +	struct iio_dev *indio_dev;
> +	u8 part_id, rev_id, seq_id;
> +	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;
> +	data->part_info = &si1145_part_info[id->driver_data];
> +
> +	part_id = ret = i2c_smbus_read_byte_data(data->client, SI1145_REG_PART_ID);
> +	if (ret < 0)
> +		return ret;
> +	rev_id = ret = i2c_smbus_read_byte_data(data->client, SI1145_REG_REV_ID);
> +	if (ret < 0)
> +		return ret;
> +	seq_id = ret = i2c_smbus_read_byte_data(data->client, SI1145_REG_SEQ_ID);
> +	if (ret < 0)
> +		return ret;
> +	dev_info(&client->dev, "Device ID part %#02hhx rev %#02hhx seq %#02hhx\n",
> +			part_id, rev_id, seq_id);
> +	if (part_id != data->part_info->part) {
> +		dev_err(&client->dev, "part ID mismatch got %#02hhx expected %#02x\n",
> +				part_id, data->part_info->part);
> +		return -ENODEV;
> +	}
> +
> +	indio_dev->dev.parent = &client->dev;
> +	indio_dev->name = id->name;
> +	indio_dev->channels = data->part_info->channels;
> +	indio_dev->num_channels = data->part_info->num_channels;
> +	indio_dev->info = data->part_info->iio_info;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +
> +	mutex_init(&data->lock);
> +	mutex_init(&data->cmdlock);
> +
> +	ret = si1145_initialize(data);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = iio_triggered_buffer_setup(indio_dev, NULL,
> +		si1145_trigger_handler, &si1145_buffer_setup_ops);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (client->irq) {
> +		ret = si1145_probe_trigger(indio_dev);
> +		if (ret < 0)
> +			goto error_free_buffer;
> +
> +		ret = devm_request_irq(&client->dev,
> +				       client->irq,
> +				       iio_trigger_generic_data_rdy_poll,
> +				       IRQF_TRIGGER_FALLING,
> +				       "si1145_irq",
> +				       data->trig);
I'm not so keen on using devm here as it breaks the obviously correct rule.
You will end up unwinding the probe_trigger, before you unwind this
rather than the other way around.  I know it's 'almost certainly fine'
but for a tiny bit more code, let's go with certainly ;)
> +		if (ret < 0) {
> +			dev_err(&client->dev, "irq request failed\n");
> +			goto error_free_trigger;
> +		}
> +	} else {
> +		dev_info(&client->dev, "no irq, using polling\n");
> +	}
> +
> +	ret = iio_device_register(indio_dev);
> +	if (ret < 0)
> +		goto error_free_trigger;
> +
> +	return 0;
> +
> +error_free_trigger:
> +	si1145_remove_trigger(indio_dev);
> +error_free_buffer:
> +	iio_triggered_buffer_cleanup(indio_dev);
> +	return ret;
> +}
> +
> +static const struct i2c_device_id si1145_ids[] = {
> +	{ "si1132", SI1132 },
> +	{ "si1141", SI1141 },
> +	{ "si1142", SI1142 },
> +	{ "si1143", SI1143 },
> +	{ "si1145", SI1145 },
> +	{ "si1146", SI1146 },
> +	{ "si1147", SI1147 },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(i2c, si1145_ids);
> +
> +static int si1145_remove(struct i2c_client *client)
> +{
> +	struct iio_dev *indio_dev = i2c_get_clientdata(client);
> +
> +	iio_device_unregister(indio_dev);
> +	iio_triggered_buffer_cleanup(indio_dev);
> +	si1145_remove_trigger(indio_dev);
> +
> +	return 0;
> +}
> +
> +static struct i2c_driver si1145_driver = {
> +	.driver = {
> +		.name   = "si1145",
> +		.owner  = THIS_MODULE,
> +	},
> +	.probe  = si1145_probe,
> +	.remove = si1145_remove,
> +	.id_table = si1145_ids,
> +};
> +
> +module_i2c_driver(si1145_driver);
> +
> +MODULE_AUTHOR("Peter Meerwald-Stadler <pmeerw@...erw.net>");
> +MODULE_DESCRIPTION("Silabs SI1132 and SI1141/2/3/5/6/7 proximity, ambient light and UV index sensor driver");
> +MODULE_LICENSE("GPL");
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ