On Sat, 1 Jul 2017 13:29:10 +0100 Jonathan Cameron <jic23@xxxxxxxxxx> wrote: > On Tue, 27 Jun 2017 00:44:55 +0900 > Akinobu Mita <akinobu.mita@xxxxxxxxx> wrote: > > > The TCS3472 device provides interrupt signal for out-of-threshold events > > with persistence filter. > > > > This change adds interrupt support for the threshold events and enables > > to configure the period of time by persistence filter. > > > > Cc: Peter Meerwald <pmeerw@xxxxxxxxxx> > > Cc: Jonathan Cameron <jic23@xxxxxxxxxx> > > Signed-off-by: Akinobu Mita <akinobu.mita@xxxxxxxxx> > Looks fine to me, but I'd like to leave a little longer for Peter to > comment if he wishes to. Also Jon might have some input. Guess neither of them has enough time. Applied to the togreg branch of iio.git and pushed out as testing for the autobuilders to play with it. Thanks, Jonathan > > Jonathan > > --- > > * Changes from v1 (addressed comment from Peter Meerwald) > > - Neaten the special function code notation > > - Improve the code comment for tcs3472_intr_pers[] > > - Fix error path with invalid event info in write_event_value() > > - Handle correctly if the negative period time is specified (round to the > > minimum available period time) > > - Remove redundant the PERS register write > > - Call free_irq() in reverse order of the initialization. > > > > drivers/iio/light/tcs3472.c | 262 ++++++++++++++++++++++++++++++++++++++++++-- > > 1 file changed, 254 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/iio/light/tcs3472.c b/drivers/iio/light/tcs3472.c > > index 09e6ca5..e1c8a07 100644 > > --- a/drivers/iio/light/tcs3472.c > > +++ b/drivers/iio/light/tcs3472.c > > @@ -13,7 +13,7 @@ > > * > > * Datasheet: http://ams.com/eng/content/download/319364/1117183/file/TCS3472_Datasheet_EN_v2.pdf > > * > > - * TODO: interrupt support, thresholds, wait time > > + * TODO: wait time > > */ > > > > #include <linux/module.h> > > @@ -23,6 +23,7 @@ > > > > #include <linux/iio/iio.h> > > #include <linux/iio/sysfs.h> > > +#include <linux/iio/events.h> > > #include <linux/iio/trigger_consumer.h> > > #include <linux/iio/buffer.h> > > #include <linux/iio/triggered_buffer.h> > > @@ -31,12 +32,15 @@ > > > > #define TCS3472_COMMAND BIT(7) > > #define TCS3472_AUTO_INCR BIT(5) > > +#define TCS3472_SPECIAL_FUNC (BIT(5) | BIT(6)) > > + > > +#define TCS3472_INTR_CLEAR (TCS3472_COMMAND | TCS3472_SPECIAL_FUNC | 0x06) > > > > #define TCS3472_ENABLE (TCS3472_COMMAND | 0x00) > > #define TCS3472_ATIME (TCS3472_COMMAND | 0x01) > > #define TCS3472_WTIME (TCS3472_COMMAND | 0x03) > > -#define TCS3472_AILT (TCS3472_COMMAND | 0x04) > > -#define TCS3472_AIHT (TCS3472_COMMAND | 0x06) > > +#define TCS3472_AILT (TCS3472_COMMAND | TCS3472_AUTO_INCR | 0x04) > > +#define TCS3472_AIHT (TCS3472_COMMAND | TCS3472_AUTO_INCR | 0x06) > > #define TCS3472_PERS (TCS3472_COMMAND | 0x0c) > > #define TCS3472_CONFIG (TCS3472_COMMAND | 0x0d) > > #define TCS3472_CONTROL (TCS3472_COMMAND | 0x0f) > > @@ -47,19 +51,42 @@ > > #define TCS3472_GDATA (TCS3472_COMMAND | TCS3472_AUTO_INCR | 0x18) > > #define TCS3472_BDATA (TCS3472_COMMAND | TCS3472_AUTO_INCR | 0x1a) > > > > +#define TCS3472_STATUS_AINT BIT(4) > > #define TCS3472_STATUS_AVALID BIT(0) > > +#define TCS3472_ENABLE_AIEN BIT(4) > > #define TCS3472_ENABLE_AEN BIT(1) > > #define TCS3472_ENABLE_PON BIT(0) > > #define TCS3472_CONTROL_AGAIN_MASK (BIT(0) | BIT(1)) > > > > struct tcs3472_data { > > struct i2c_client *client; > > + struct mutex lock; > > + u16 low_thresh; > > + u16 high_thresh; > > u8 enable; > > u8 control; > > u8 atime; > > + u8 apers; > > u16 buffer[8]; /* 4 16-bit channels + 64-bit timestamp */ > > }; > > > > +static const struct iio_event_spec tcs3472_events[] = { > > + { > > + .type = IIO_EV_TYPE_THRESH, > > + .dir = IIO_EV_DIR_RISING, > > + .mask_separate = BIT(IIO_EV_INFO_VALUE), > > + }, { > > + .type = IIO_EV_TYPE_THRESH, > > + .dir = IIO_EV_DIR_FALLING, > > + .mask_separate = BIT(IIO_EV_INFO_VALUE), > > + }, { > > + .type = IIO_EV_TYPE_THRESH, > > + .dir = IIO_EV_DIR_EITHER, > > + .mask_separate = BIT(IIO_EV_INFO_ENABLE) | > > + BIT(IIO_EV_INFO_PERIOD), > > + }, > > +}; > > + > > #define TCS3472_CHANNEL(_color, _si, _addr) { \ > > .type = IIO_INTENSITY, \ > > .modified = 1, \ > > @@ -75,6 +102,8 @@ struct tcs3472_data { > > .storagebits = 16, \ > > .endianness = IIO_CPU, \ > > }, \ > > + .event_spec = _si ? NULL : tcs3472_events, \ > > + .num_event_specs = _si ? 0 : ARRAY_SIZE(tcs3472_events), \ > > } > > > > static const int tcs3472_agains[] = { 1, 4, 16, 60 }; > > @@ -182,6 +211,166 @@ static int tcs3472_write_raw(struct iio_dev *indio_dev, > > return -EINVAL; > > } > > > > +/* > > + * Translation from APERS field value to the number of consecutive out-of-range > > + * clear channel values before an interrupt is generated > > + */ > > +static const int tcs3472_intr_pers[] = { > > + 0, 1, 2, 3, 5, 10, 15, 20, 25, 30, 35, 40, 45, 50, 55, 60 > > +}; > > + > > +static int tcs3472_read_event(struct iio_dev *indio_dev, > > + const struct iio_chan_spec *chan, enum iio_event_type type, > > + enum iio_event_direction dir, enum iio_event_info info, int *val, > > + int *val2) > > +{ > > + struct tcs3472_data *data = iio_priv(indio_dev); > > + int ret; > > + unsigned int period; > > + > > + mutex_lock(&data->lock); > > + > > + switch (info) { > > + case IIO_EV_INFO_VALUE: > > + *val = (dir == IIO_EV_DIR_RISING) ? > > + data->high_thresh : data->low_thresh; > > + ret = IIO_VAL_INT; > > + break; > > + case IIO_EV_INFO_PERIOD: > > + period = (256 - data->atime) * 2400 * > > + tcs3472_intr_pers[data->apers]; > > + *val = period / USEC_PER_SEC; > > + *val2 = period % USEC_PER_SEC; > > + ret = IIO_VAL_INT_PLUS_MICRO; > > + break; > > + default: > > + ret = -EINVAL; > > + break; > > + } > > + > > + mutex_unlock(&data->lock); > > + > > + return ret; > > +} > > + > > +static int tcs3472_write_event(struct iio_dev *indio_dev, > > + const struct iio_chan_spec *chan, enum iio_event_type type, > > + enum iio_event_direction dir, enum iio_event_info info, int val, > > + int val2) > > +{ > > + struct tcs3472_data *data = iio_priv(indio_dev); > > + int ret; > > + u8 command; > > + int period; > > + int i; > > + > > + mutex_lock(&data->lock); > > + switch (info) { > > + case IIO_EV_INFO_VALUE: > > + switch (dir) { > > + case IIO_EV_DIR_RISING: > > + command = TCS3472_AIHT; > > + break; > > + case IIO_EV_DIR_FALLING: > > + command = TCS3472_AILT; > > + break; > > + default: > > + ret = -EINVAL; > > + goto error; > > + } > > + ret = i2c_smbus_write_word_data(data->client, command, val); > > + if (ret) > > + goto error; > > + > > + if (dir == IIO_EV_DIR_RISING) > > + data->high_thresh = val; > > + else > > + data->low_thresh = val; > > + break; > > + case IIO_EV_INFO_PERIOD: > > + period = val * USEC_PER_SEC + val2; > > + for (i = 1; i < ARRAY_SIZE(tcs3472_intr_pers) - 1; i++) { > > + if (period <= (256 - data->atime) * 2400 * > > + tcs3472_intr_pers[i]) > > + break; > > + } > > + ret = i2c_smbus_write_byte_data(data->client, TCS3472_PERS, i); > > + if (ret) > > + goto error; > > + > > + data->apers = i; > > + break; > > + default: > > + ret = -EINVAL; > > + break; > > + } > > +error: > > + mutex_unlock(&data->lock); > > + > > + return ret; > > +} > > + > > +static int tcs3472_read_event_config(struct iio_dev *indio_dev, > > + const struct iio_chan_spec *chan, enum iio_event_type type, > > + enum iio_event_direction dir) > > +{ > > + struct tcs3472_data *data = iio_priv(indio_dev); > > + int ret; > > + > > + mutex_lock(&data->lock); > > + ret = !!(data->enable & TCS3472_ENABLE_AIEN); > > + mutex_unlock(&data->lock); > > + > > + return ret; > > +} > > + > > +static int tcs3472_write_event_config(struct iio_dev *indio_dev, > > + const struct iio_chan_spec *chan, enum iio_event_type type, > > + enum iio_event_direction dir, int state) > > +{ > > + struct tcs3472_data *data = iio_priv(indio_dev); > > + int ret = 0; > > + u8 enable_old; > > + > > + mutex_lock(&data->lock); > > + > > + enable_old = data->enable; > > + > > + if (state) > > + data->enable |= TCS3472_ENABLE_AIEN; > > + else > > + data->enable &= ~TCS3472_ENABLE_AIEN; > > + > > + if (enable_old != data->enable) { > > + ret = i2c_smbus_write_byte_data(data->client, TCS3472_ENABLE, > > + data->enable); > > + if (ret) > > + data->enable = enable_old; > > + } > > + mutex_unlock(&data->lock); > > + > > + return ret; > > +} > > + > > +static irqreturn_t tcs3472_event_handler(int irq, void *priv) > > +{ > > + struct iio_dev *indio_dev = priv; > > + struct tcs3472_data *data = iio_priv(indio_dev); > > + int ret; > > + > > + ret = i2c_smbus_read_byte_data(data->client, TCS3472_STATUS); > > + if (ret >= 0 && (ret & TCS3472_STATUS_AINT)) { > > + iio_push_event(indio_dev, IIO_UNMOD_EVENT_CODE(IIO_INTENSITY, 0, > > + IIO_EV_TYPE_THRESH, > > + IIO_EV_DIR_EITHER), > > + iio_get_time_ns(indio_dev)); > > + > > + i2c_smbus_read_byte_data(data->client, TCS3472_INTR_CLEAR); > > + } > > + > > + return IRQ_HANDLED; > > +} > > + > > static irqreturn_t tcs3472_trigger_handler(int irq, void *p) > > { > > struct iio_poll_func *pf = p; > > @@ -245,6 +434,10 @@ static const struct attribute_group tcs3472_attribute_group = { > > static const struct iio_info tcs3472_info = { > > .read_raw = tcs3472_read_raw, > > .write_raw = tcs3472_write_raw, > > + .read_event_value = tcs3472_read_event, > > + .write_event_value = tcs3472_write_event, > > + .read_event_config = tcs3472_read_event_config, > > + .write_event_config = tcs3472_write_event_config, > > .attrs = &tcs3472_attribute_group, > > .driver_module = THIS_MODULE, > > }; > > @@ -263,6 +456,7 @@ static int tcs3472_probe(struct i2c_client *client, > > data = iio_priv(indio_dev); > > i2c_set_clientdata(client, indio_dev); > > data->client = client; > > + mutex_init(&data->lock); > > > > indio_dev->dev.parent = &client->dev; > > indio_dev->info = &tcs3472_info; > > @@ -292,12 +486,29 @@ static int tcs3472_probe(struct i2c_client *client, > > return ret; > > data->atime = ret; > > > > + ret = i2c_smbus_read_word_data(data->client, TCS3472_AILT); > > + if (ret < 0) > > + return ret; > > + data->low_thresh = ret; > > + > > + ret = i2c_smbus_read_word_data(data->client, TCS3472_AIHT); > > + if (ret < 0) > > + return ret; > > + data->high_thresh = ret; > > + > > + data->apers = 1; > > + ret = i2c_smbus_write_byte_data(data->client, TCS3472_PERS, > > + data->apers); > > + if (ret < 0) > > + return ret; > > + > > ret = i2c_smbus_read_byte_data(data->client, TCS3472_ENABLE); > > if (ret < 0) > > return ret; > > > > /* enable device */ > > data->enable = ret | TCS3472_ENABLE_PON | TCS3472_ENABLE_AEN; > > + data->enable &= ~TCS3472_ENABLE_AIEN; > > ret = i2c_smbus_write_byte_data(data->client, TCS3472_ENABLE, > > data->enable); > > if (ret < 0) > > @@ -308,12 +519,24 @@ static int tcs3472_probe(struct i2c_client *client, > > if (ret < 0) > > return ret; > > > > + if (client->irq) { > > + ret = request_threaded_irq(client->irq, NULL, > > + tcs3472_event_handler, > > + IRQF_TRIGGER_FALLING | IRQF_SHARED | > > + IRQF_ONESHOT, > > + client->name, indio_dev); > > + if (ret) > > + goto buffer_cleanup; > > + } > > + > > ret = iio_device_register(indio_dev); > > if (ret < 0) > > - goto buffer_cleanup; > > + goto free_irq; > > > > return 0; > > > > +free_irq: > > + free_irq(client->irq, indio_dev); > > buffer_cleanup: > > iio_triggered_buffer_cleanup(indio_dev); > > return ret; > > @@ -321,8 +544,19 @@ static int tcs3472_probe(struct i2c_client *client, > > > > static int tcs3472_powerdown(struct tcs3472_data *data) > > { > > - return i2c_smbus_write_byte_data(data->client, TCS3472_ENABLE, > > - data->enable & ~(TCS3472_ENABLE_AEN | TCS3472_ENABLE_PON)); > > + int ret; > > + u8 enable_mask = TCS3472_ENABLE_AEN | TCS3472_ENABLE_PON; > > + > > + mutex_lock(&data->lock); > > + > > + ret = i2c_smbus_write_byte_data(data->client, TCS3472_ENABLE, > > + data->enable & ~enable_mask); > > + if (!ret) > > + data->enable &= ~enable_mask; > > + > > + mutex_unlock(&data->lock); > > + > > + return ret; > > } > > > > static int tcs3472_remove(struct i2c_client *client) > > @@ -330,6 +564,7 @@ static int tcs3472_remove(struct i2c_client *client) > > struct iio_dev *indio_dev = i2c_get_clientdata(client); > > > > iio_device_unregister(indio_dev); > > + free_irq(client->irq, indio_dev); > > iio_triggered_buffer_cleanup(indio_dev); > > tcs3472_powerdown(iio_priv(indio_dev)); > > > > @@ -348,8 +583,19 @@ static int tcs3472_resume(struct device *dev) > > { > > struct tcs3472_data *data = iio_priv(i2c_get_clientdata( > > to_i2c_client(dev))); > > - return i2c_smbus_write_byte_data(data->client, TCS3472_ENABLE, > > - data->enable | (TCS3472_ENABLE_AEN | TCS3472_ENABLE_PON)); > > + int ret; > > + u8 enable_mask = TCS3472_ENABLE_AEN | TCS3472_ENABLE_PON; > > + > > + mutex_lock(&data->lock); > > + > > + ret = i2c_smbus_write_byte_data(data->client, TCS3472_ENABLE, > > + data->enable | enable_mask); > > + if (!ret) > > + data->enable |= enable_mask; > > + > > + mutex_unlock(&data->lock); > > + > > + return ret; > > } > > #endif > > > > -- > 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 -- 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
