Re: [PATCH v3] iio: Add driver for Silabs si1132, si1141/2/3 and si1145/6/7 ambient light, uv index and proximity sensors
From: Jonathan Cameron
Date: Sun Jun 19 2016 - 07:57:25 EST
On 17/06/16 12:10, Crestez Dan Leonard wrote:
> From: Peter Meerwald <pmeerw@xxxxxxxxxx>
>
> 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
>
> Changes since v2:
> * Add myself to copyright
> * Remove TODO: power management.
> It's fine because by default the device remains sleeping.
> * Elaborate comments on si1145_data
> * Adjust comment syntax
> * Drop special case for reading single words.
> * Use manual request_irq to ensure proper init/free ordering
>
> 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 was tested on si1143 and si1145
>
> Link to v2: https://www.spinics.net/lists/linux-iio/msg25055.html
>
Can you explain what the autonomous stuff is about? Has me confused.
Is it an attempt to allow driving the device with a software trigger
when no interrupt is provided? (I think it is but correct me if I'm
wrong). I'm definitely not happy with this approach as a trigger 'in'
should always result in data 'out'. If you are running multiple
sensors off one software trigger you'd expect to get the same amount of
data from each.
I think the right way to do this is provide a thread based sampler of
the irq status register inside the driver and hence have it run off a
normal trigger.. It's not a device where running off any other trigger
makes much sense (I think...)
If you want to look into it, I'd like to see if some 'library code'
to assist with usecase is a worthwhile exercise.
We always end up spinning up a thread. This thread then calls some
trivial 'new data ready' query from the hardware and fires the trigger
if there is. Thread frequency is driven by the expected change
frequency (in simple case, it's double it).
It's not a lot of code in any case, but if we provide a library version
then we can at least ensure all drivers who want this support (which
is basically any with a dataready interrupt that may not be wired) do
it the same way.
I've raised a question in response to an earlier thread on names from
device tree to try and clarify one other point inline.
Mostly looking good, just a few loose ends to clean up.
Jonathan
> Signed-off-by: Peter Meerwald <pmeerw@xxxxxxxxxx>
> Signed-off-by: Crestez Dan Leonard <leonard.crestez@xxxxxxxxx>
> ---
> drivers/iio/light/Kconfig | 13 +
> drivers/iio/light/Makefile | 1 +
> drivers/iio/light/si1145.c | 1347 ++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 1361 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..e82fb4b
> --- /dev/null
> +++ b/drivers/iio/light/si1145.c
> @@ -0,0 +1,1347 @@
> +/*
> + * 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@xxxxxxxxxx>
> + * Copyright 2016 Crestez Dan Leonard <leonard.crestez@xxxxxxxxx>
> + *
> + * 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)
> + */
> +
> +#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
> + * @lock: mutex to protect shared state.
> + * @cmdlock: Low-level mutex to protect command execution only
> + * @rsp_seq: Next expected response number or -1 if counter reset required
> + * @scan_mask: Saved scan mask to avoid duplicate set_chlist
> + * @autonomous: If automatic measurements are active (for buffer support)
> + * @part_info: Part information
> + * @trig: Pointer to iio trigger
> + * @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;
> +};
> +
> +/**
> + * __si1145_command_reset() - Send CMD_NOP and wait for response 0
> + *
> + * Does not modify data->rsp_seq
> + *
> + * Return: 0 on success and -errno on error.
> + */
> +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;
> + }
> +}
> +
> +/**
> + * si1145_command() - Execute a command and poll the response register
> + *
> + * All conversion overflows are reported as -EOVERFLOW
> + * INVALID_SETTING is reported as -EINVAL
> + * Timeouts are reported as -ETIMEDOUT
> + *
> + * Return: 0 on success or -errno on failure
> + */
> +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 value to 8 bit, 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
> + */
> + 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++;
> + }
> +
> + ret = i2c_smbus_read_i2c_block_data_or_emulated(
> + data->client, indio_dev->channels[i].address,
> + sizeof(u16) * run, &buffer[j]);
> + 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;
We now have nice wrapper functions for this...
iio_device_claim_direct_mode. If nothing else you should be
holding mlock here to avoild switching to buffered mode mid
way through this function call. The claim_direct stuff does
all that neatly and correctly.
> +
> + 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);
Wrapping is a little agressive here. Plenty of space to align the parameters
with the brackets...
I'd also make it a little more apparent what is going on in these with
a couple of defines to handle the break up into the two separate fields.
> + 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);
Umm. Just glancing at datasheet, that's an undocumented bit. So a doubly
magic value. If setting bits in a register like this, defines for all the
relevant parts will make it more obvious what is going on.
> + 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);
Again, a bit of macro magic will make it easier to tell at glance what is
going on here.
> + if (ret < 0)
> + return ret;
> +
> + /* Set ALS visible measurement mode */
> + ret = si1145_param_set(data, SI1145_PARAM_ALSVIS_ADC_MISC, SI1145_ADC_MISC_RANGE);
This line is too long for no particularly reason :)
> + 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,
> +};
> +
> +/**
> + * si1145_trigger_set_state() - Set trigger state
> + *
> + * 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 */
It's a worth aim, but I'd just be inclined to bomb out with the error.
This should never happen except when we have a hardware fault so I'd
not worry 'too' much about elegantly stopping when it does...
Still I don't really mind as this doesn't signicantly increase the code
complexity.
> + 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);
> + struct i2c_client *client = data->client;
> + 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;
> +
> + ret = request_irq(client->irq,
> + iio_trigger_generic_data_rdy_poll,
> + IRQF_TRIGGER_FALLING,
> + "si1145_irq",
> + data->trig);
> + if (ret < 0) {
> + dev_err(&client->dev, "irq request failed\n");
> + iio_trigger_unregister(data->trig);
> + return ret;
> + }
> +
> + 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);
> + struct i2c_client *client = data->client;
> +
> + if (data->trig) {
> + free_irq(client->irq, data->trig);
> + iio_trigger_unregister(data->trig);
> + data->trig = NULL;
> + }
> +}
> +
> +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;
Hmm. I've just sent an email in reply to a previous thread to see if we
can clarify what the right option is when we have this instantiated from
the device_tree (which can happen even without an of_device_id table)..
> + 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;
> + } 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);
> +
> + si1145_remove_trigger(indio_dev);
> + iio_triggered_buffer_cleanup(indio_dev);
> + iio_device_unregister(indio_dev);
> +
> + return 0;
> +}
> +
> +static struct i2c_driver si1145_driver = {
> + .driver = {
> + .name = "si1145",
> + .owner = THIS_MODULE,
If this is only issue I'll drop it as per the autobuilder warnings...
> + },
> + .probe = si1145_probe,
> + .remove = si1145_remove,
> + .id_table = si1145_ids,
> +};
> +
> +module_i2c_driver(si1145_driver);
> +
> +MODULE_AUTHOR("Peter Meerwald-Stadler <pmeerw@xxxxxxxxxx>");
> +MODULE_DESCRIPTION("Silabs SI1132 and SI1141/2/3/5/6/7 proximity, ambient light and UV index sensor driver");
> +MODULE_LICENSE("GPL");
>