* [PATCH 0/2] iio: light: tcs3472: bug fix and iio event handling support @ 2017-06-12 15:05 Akinobu Mita 2017-06-12 15:05 ` [PATCH 1/2] iio: light: tcs3472: fix ATIME register write Akinobu Mita 2017-06-12 15:05 ` [PATCH 2/2] iio: light: tcs3472: support out-of-threshold events Akinobu Mita 0 siblings, 2 replies; 7+ messages in thread From: Akinobu Mita @ 2017-06-12 15:05 UTC (permalink / raw) To: linux-iio; +Cc: Akinobu Mita, Peter Meerwald, Jonathan Cameron This contains a single bug fix and support iio event handling for tcs3472 driver. Akinobu Mita (2): iio: light: tcs3472: fix ATIME register write iio: light: tcs3472: support out-of-threshold events drivers/iio/light/tcs3472.c | 271 ++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 262 insertions(+), 9 deletions(-) Cc: Peter Meerwald <pmeerw@pmeerw.net> Cc: Jonathan Cameron <jic23@kernel.org> -- 2.7.4 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] iio: light: tcs3472: fix ATIME register write 2017-06-12 15:05 [PATCH 0/2] iio: light: tcs3472: bug fix and iio event handling support Akinobu Mita @ 2017-06-12 15:05 ` Akinobu Mita 2017-06-21 19:29 ` Jonathan Cameron 2017-06-12 15:05 ` [PATCH 2/2] iio: light: tcs3472: support out-of-threshold events Akinobu Mita 1 sibling, 1 reply; 7+ messages in thread From: Akinobu Mita @ 2017-06-12 15:05 UTC (permalink / raw) To: linux-iio; +Cc: Akinobu Mita, Peter Meerwald, Jonathan Cameron The integration time is controlled by the ATIME register only. However, this register is written by i2c_smbus_write_word_data() in write_raw(). We actually don't need to write a subsequent register. So just use i2c_smbus_write_byte_data() instead. Cc: Peter Meerwald <pmeerw@pmeerw.net> Cc: Jonathan Cameron <jic23@kernel.org> Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com> --- drivers/iio/light/tcs3472.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/iio/light/tcs3472.c b/drivers/iio/light/tcs3472.c index 3aa71e3..a9e153b 100644 --- a/drivers/iio/light/tcs3472.c +++ b/drivers/iio/light/tcs3472.c @@ -169,7 +169,7 @@ static int tcs3472_write_raw(struct iio_dev *indio_dev, for (i = 0; i < 256; i++) { if (val2 == (256 - i) * 2400) { data->atime = i; - return i2c_smbus_write_word_data( + return i2c_smbus_write_byte_data( data->client, TCS3472_ATIME, data->atime); } -- 2.7.4 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] iio: light: tcs3472: fix ATIME register write 2017-06-12 15:05 ` [PATCH 1/2] iio: light: tcs3472: fix ATIME register write Akinobu Mita @ 2017-06-21 19:29 ` Jonathan Cameron 2017-06-21 20:38 ` Peter Meerwald-Stadler 0 siblings, 1 reply; 7+ messages in thread From: Jonathan Cameron @ 2017-06-21 19:29 UTC (permalink / raw) To: Akinobu Mita; +Cc: linux-iio, Peter Meerwald On Tue, 13 Jun 2017 00:05:08 +0900 Akinobu Mita <akinobu.mita@gmail.com> wrote: > The integration time is controlled by the ATIME register only. However, > this register is written by i2c_smbus_write_word_data() in write_raw(). > > We actually don't need to write a subsequent register. So just use > i2c_smbus_write_byte_data() instead. > > Cc: Peter Meerwald <pmeerw@pmeerw.net> > Cc: Jonathan Cameron <jic23@kernel.org> > Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com> As I read the datasheet, this looks like it won't cause any actual harm (where the top byte is written is unused). Hence I'm going to apply this as part of the normal merge path rather than as a fix as such. Well worth cleaning up though! Applied to the togreg branch of iio.git and pushed out as testing for the autobuilders to play with it. Thanks, Jonathan > --- > drivers/iio/light/tcs3472.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/iio/light/tcs3472.c b/drivers/iio/light/tcs3472.c > index 3aa71e3..a9e153b 100644 > --- a/drivers/iio/light/tcs3472.c > +++ b/drivers/iio/light/tcs3472.c > @@ -169,7 +169,7 @@ static int tcs3472_write_raw(struct iio_dev *indio_dev, > for (i = 0; i < 256; i++) { > if (val2 == (256 - i) * 2400) { > data->atime = i; > - return i2c_smbus_write_word_data( > + return i2c_smbus_write_byte_data( > data->client, TCS3472_ATIME, > data->atime); > } ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] iio: light: tcs3472: fix ATIME register write 2017-06-21 19:29 ` Jonathan Cameron @ 2017-06-21 20:38 ` Peter Meerwald-Stadler 0 siblings, 0 replies; 7+ messages in thread From: Peter Meerwald-Stadler @ 2017-06-21 20:38 UTC (permalink / raw) To: Jonathan Cameron; +Cc: Akinobu Mita, linux-iio > > The integration time is controlled by the ATIME register only. However, > > this register is written by i2c_smbus_write_word_data() in write_raw(). > > > > We actually don't need to write a subsequent register. So just use > > i2c_smbus_write_byte_data() instead. > > Cc: Peter Meerwald <pmeerw@pmeerw.net> > > Cc: Jonathan Cameron <jic23@kernel.org> > > Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com> > As I read the datasheet, this looks like it won't cause any actual > harm (where the top byte is written is unused). looks good to me as well, Acked-by: Peter Meerwald-Stadler <pmeerw@pmeerw.net> > > --- > > drivers/iio/light/tcs3472.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/iio/light/tcs3472.c b/drivers/iio/light/tcs3472.c > > index 3aa71e3..a9e153b 100644 > > --- a/drivers/iio/light/tcs3472.c > > +++ b/drivers/iio/light/tcs3472.c > > @@ -169,7 +169,7 @@ static int tcs3472_write_raw(struct iio_dev *indio_dev, > > for (i = 0; i < 256; i++) { > > if (val2 == (256 - i) * 2400) { > > data->atime = i; > > - return i2c_smbus_write_word_data( > > + return i2c_smbus_write_byte_data( > > data->client, TCS3472_ATIME, > > data->atime); > > } > -- Peter Meerwald-Stadler Mobile: +43 664 24 44 418 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/2] iio: light: tcs3472: support out-of-threshold events 2017-06-12 15:05 [PATCH 0/2] iio: light: tcs3472: bug fix and iio event handling support Akinobu Mita 2017-06-12 15:05 ` [PATCH 1/2] iio: light: tcs3472: fix ATIME register write Akinobu Mita @ 2017-06-12 15:05 ` Akinobu Mita 2017-06-21 19:33 ` Jonathan Cameron 1 sibling, 1 reply; 7+ messages in thread From: Akinobu Mita @ 2017-06-12 15:05 UTC (permalink / raw) To: linux-iio; +Cc: Akinobu Mita, Peter Meerwald, Jonathan Cameron 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@pmeerw.net> Cc: Jonathan Cameron <jic23@kernel.org> Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com> --- drivers/iio/light/tcs3472.c | 269 ++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 261 insertions(+), 8 deletions(-) diff --git a/drivers/iio/light/tcs3472.c b/drivers/iio/light/tcs3472.c index a9e153b..905e777 100644 --- a/drivers/iio/light/tcs3472.c +++ b/drivers/iio/light/tcs3472.c @@ -11,7 +11,9 @@ * 7-bit I2C slave address 0x39 (TCS34721, TCS34723) or 0x29 (TCS34725, * TCS34727) * - * TODO: interrupt support, thresholds, wait time + * Datasheet: http://ams.com/eng/content/download/319364/1117183/file/TCS3472_Datasheet_EN_v2.pdf + * + * TODO: wait time */ #include <linux/module.h> @@ -21,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> @@ -29,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 | 0x6) #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) @@ -45,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, \ @@ -73,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 }; @@ -180,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 occurrences 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; + unsigned 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; + break; + } + 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; @@ -243,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, }; @@ -261,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; @@ -290,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) @@ -306,12 +519,29 @@ static int tcs3472_probe(struct i2c_client *client, if (ret < 0) return ret; + if (client->irq) { + ret = i2c_smbus_write_byte_data(data->client, TCS3472_PERS, + data->apers); + if (ret) + goto buffer_cleanup; + + 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; @@ -319,14 +549,26 @@ 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) { struct iio_dev *indio_dev = i2c_get_clientdata(client); + free_irq(client->irq, indio_dev); iio_device_unregister(indio_dev); iio_triggered_buffer_cleanup(indio_dev); tcs3472_powerdown(iio_priv(indio_dev)); @@ -346,8 +588,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 -- 2.7.4 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] iio: light: tcs3472: support out-of-threshold events 2017-06-12 15:05 ` [PATCH 2/2] iio: light: tcs3472: support out-of-threshold events Akinobu Mita @ 2017-06-21 19:33 ` Jonathan Cameron 2017-06-21 20:38 ` Peter Meerwald-Stadler 0 siblings, 1 reply; 7+ messages in thread From: Jonathan Cameron @ 2017-06-21 19:33 UTC (permalink / raw) To: Akinobu Mita; +Cc: linux-iio, Peter Meerwald On Tue, 13 Jun 2017 00:05:09 +0900 Akinobu Mita <akinobu.mita@gmail.com> 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@pmeerw.net> > Cc: Jonathan Cameron <jic23@kernel.org> > Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com> One comment inline. Looks fine to me, but I'd like to give Peter more time to have a look at it. If we don't hear from Peter within two weeks (ish) give me a bump and I'll either chase up or take the view Peter is too busy. Jonathan > --- > drivers/iio/light/tcs3472.c | 269 ++++++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 261 insertions(+), 8 deletions(-) > > diff --git a/drivers/iio/light/tcs3472.c b/drivers/iio/light/tcs3472.c > index a9e153b..905e777 100644 > --- a/drivers/iio/light/tcs3472.c > +++ b/drivers/iio/light/tcs3472.c > @@ -11,7 +11,9 @@ > * 7-bit I2C slave address 0x39 (TCS34721, TCS34723) or 0x29 (TCS34725, > * TCS34727) > * > - * TODO: interrupt support, thresholds, wait time > + * Datasheet: http://ams.com/eng/content/download/319364/1117183/file/TCS3472_Datasheet_EN_v2.pdf I'd have have marginally preferred this change as a separate patch, but if everything else is fine I don't care enough to delay this. > + * > + * TODO: wait time > */ > > #include <linux/module.h> > @@ -21,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> > @@ -29,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 | 0x6) > > #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) > @@ -45,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, \ > @@ -73,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 }; > @@ -180,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 occurrences 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; > + unsigned 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; > + break; > + } > + 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; > @@ -243,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, > }; > @@ -261,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; > @@ -290,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) > @@ -306,12 +519,29 @@ static int tcs3472_probe(struct i2c_client *client, > if (ret < 0) > return ret; > > + if (client->irq) { > + ret = i2c_smbus_write_byte_data(data->client, TCS3472_PERS, > + data->apers); > + if (ret) > + goto buffer_cleanup; > + > + 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; > @@ -319,14 +549,26 @@ 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) > { > struct iio_dev *indio_dev = i2c_get_clientdata(client); > > + free_irq(client->irq, indio_dev); > iio_device_unregister(indio_dev); > iio_triggered_buffer_cleanup(indio_dev); > tcs3472_powerdown(iio_priv(indio_dev)); > @@ -346,8 +588,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 > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] iio: light: tcs3472: support out-of-threshold events 2017-06-21 19:33 ` Jonathan Cameron @ 2017-06-21 20:38 ` Peter Meerwald-Stadler 0 siblings, 0 replies; 7+ messages in thread From: Peter Meerwald-Stadler @ 2017-06-21 20:38 UTC (permalink / raw) To: Jonathan Cameron; +Cc: Akinobu Mita, linux-iio > > 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@pmeerw.net> > > Cc: Jonathan Cameron <jic23@kernel.org> > > Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com> > One comment inline. Looks fine to me, but I'd like to give > Peter more time to have a look at it. my comments below, mostly minor > If we don't hear from Peter within two weeks (ish) give me a bump > and I'll either chase up or take the view Peter is too busy. > > Jonathan > > --- > > drivers/iio/light/tcs3472.c | 269 ++++++++++++++++++++++++++++++++++++++++++-- > > 1 file changed, 261 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/iio/light/tcs3472.c b/drivers/iio/light/tcs3472.c > > index a9e153b..905e777 100644 > > --- a/drivers/iio/light/tcs3472.c > > +++ b/drivers/iio/light/tcs3472.c > > @@ -11,7 +11,9 @@ > > * 7-bit I2C slave address 0x39 (TCS34721, TCS34723) or 0x29 (TCS34725, > > * TCS34727) > > * > > - * TODO: interrupt support, thresholds, wait time > > + * Datasheet: http://ams.com/eng/content/download/319364/1117183/file/TCS3472_Datasheet_EN_v2.pdf > I'd have have marginally preferred this change as a separate patch, but if everything else > is fine I don't care enough to delay this. > > + * > > + * TODO: wait time > > */ > > > > #include <linux/module.h> > > @@ -21,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> > > @@ -29,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 | 0x6) nitpick: the special function code is 5 bits, so should be 0x06 instead of 0x6 > > #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) > > @@ -45,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, \ > > @@ -73,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 }; > > @@ -180,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 occurrences before an interrupt is generated this wording could be improved, maybe 2nd line could be: * 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; > > + unsigned 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; no, it should goto error here; otherwise command is undefined in the write below > > + break; > > + } > > + 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; what if val is negative? > > + 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; > > @@ -243,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, > > }; > > @@ -261,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; > > @@ -290,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) > > @@ -306,12 +519,29 @@ static int tcs3472_probe(struct i2c_client *client, > > if (ret < 0) > > return ret; > > > > + if (client->irq) { > > + ret = i2c_smbus_write_byte_data(data->client, TCS3472_PERS, > > + data->apers); _PERS has been written before? > > + if (ret) > > + goto buffer_cleanup; > > + > > + 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; > > @@ -319,14 +549,26 @@ 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) > > { > > struct iio_dev *indio_dev = i2c_get_clientdata(client); > > > > + free_irq(client->irq, indio_dev); teardown should be in reverse order, so free_irq() should be below _unregister() > > iio_device_unregister(indio_dev); > > iio_triggered_buffer_cleanup(indio_dev); > > tcs3472_powerdown(iio_priv(indio_dev)); > > @@ -346,8 +588,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 > > > -- Peter Meerwald-Stadler Mobile: +43 664 24 44 418 ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-06-21 20:38 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-06-12 15:05 [PATCH 0/2] iio: light: tcs3472: bug fix and iio event handling support Akinobu Mita 2017-06-12 15:05 ` [PATCH 1/2] iio: light: tcs3472: fix ATIME register write Akinobu Mita 2017-06-21 19:29 ` Jonathan Cameron 2017-06-21 20:38 ` Peter Meerwald-Stadler 2017-06-12 15:05 ` [PATCH 2/2] iio: light: tcs3472: support out-of-threshold events Akinobu Mita 2017-06-21 19:33 ` Jonathan Cameron 2017-06-21 20:38 ` Peter Meerwald-Stadler
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).