[RFC PATCH 0/4] staging:iio:adis16350 various updates and fixes

[RFC PATCH 0/4] staging:iio:adis16350 various updates and fixes

[Jonathan Cameron]

The first patch fixes a bug in the current driver.  It assumes
all devices supported can do burst reads.  At least if the data
sheets are to be believed this is only available in the newer
models.

Patch 2 fixes an issue that has snuck into a number of drivers.
The use of an event for the trigger code on lis3l02dq was needed
because it uses the same physical line for datardy and threshold
interrupts (though not at the same time).  This device has two
physical lines. In my view it is reasonable to assume anyone wanting
both datardy triggering and events can select at compile time which
comes out of which pin (or if only one pin is connected, they can
set what it does.) Right now I haven't put the platform data in place
to do this.  This needs to be tested before I send this patch set
to Greg.

Patch 3 puts in event support.  It's complex, but then what the
device has some complex abilities. Still if anyone can see any
simplifications without breaking the interface I would definitely
like to hear them!

Patch 4 is trivial fix picked up by sparse whilst checking
the other patches.

Thanks,

Jonathan Cameron (4):
  staging:iio:adis16350 add non burst buffer fill and fix burst logic
  staging:iio:adis16350 move datardy trigger to straight interrupt.
  staging:iio:adis16350 Add optional event support
  staging:iio:adis16350 add missing registration of temp_offset attr

 drivers/staging/iio/adc/adc.h               |    8 ++-
 drivers/staging/iio/gyro/gyro.h             |   31 +++++++---
 drivers/staging/iio/imu/Kconfig             |    7 ++
 drivers/staging/iio/imu/Makefile            |    1 +
 drivers/staging/iio/imu/adis16350.h         |   64 +++++++++++++++++----
 drivers/staging/iio/imu/adis16350_core.c    |   54 +++++++----------
 drivers/staging/iio/imu/adis16350_ring.c    |   85 +++++++++++++++++++++++----
 drivers/staging/iio/imu/adis16350_trigger.c |   52 +++++------------
 drivers/staging/iio/sysfs.h                 |   17 +++++-
 9 files changed, 216 insertions(+), 103 deletions(-)

[PATCH 1/4] staging:iio:adis16350 add non burst buffer fill and fix burst logic

[Jonathan Cameron]

If the adis16350 etc support burst mode it is undocumented and does
not seem to work. Hence this adds an alternate read function.
It also fixes the logic for cases where not all channels are
present in the burst path.

Signed-off-by: Jonathan Cameron <jic23@cam.ac.uk>
---
 drivers/staging/iio/imu/adis16350.h      |    2 +
 drivers/staging/iio/imu/adis16350_core.c |    9 ++--
 drivers/staging/iio/imu/adis16350_ring.c |   75 +++++++++++++++++++++++++++---
 3 files changed, 75 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/iio/imu/adis16350.h b/drivers/staging/iio/imu/adis16350.h
index b00001e..07d3eeb 100644
--- a/drivers/staging/iio/imu/adis16350.h
+++ b/drivers/staging/iio/imu/adis16350.h
@@ -107,6 +107,7 @@
  * @tx:			transmit buffer
  * @rx:			recieve buffer
  * @buf_lock:		mutex to protect tx and rx
+ * @burst_available:	does the device support burst reading
  **/
 struct adis16350_state {
 	struct spi_device		*us;
@@ -117,6 +118,7 @@ struct adis16350_state {
 	u8				*tx;
 	u8				*rx;
 	struct mutex			buf_lock;
+	unsigned int			burst_available:1;
 };
 
 int adis16350_set_irq(struct device *dev, bool enable);
diff --git a/drivers/staging/iio/imu/adis16350_core.c b/drivers/staging/iio/imu/adis16350_core.c
index cc33843..d8181f9 100644
--- a/drivers/staging/iio/imu/adis16350_core.c
+++ b/drivers/staging/iio/imu/adis16350_core.c
@@ -615,6 +615,7 @@ static int __devinit adis16350_probe(struct spi_device *spi)
 	}
 	st->us = spi;
 	mutex_init(&st->buf_lock);
+	st->burst_available = spi_get_device_id(spi)->driver_data;
 	/* setup the industrialio driver allocated elements */
 	st->indio_dev = iio_allocate_device();
 	if (st->indio_dev == NULL) {
@@ -722,10 +723,10 @@ static const struct spi_device_id adis16350_id[] = {
 	{"adis16350", 0},
 	{"adis16354", 0},
 	{"adis16355", 0},
-	{"adis16360", 0},
-	{"adis16362", 0},
-	{"adis16364", 0},
-	{"adis16365", 0},
+	{"adis16360", 1},
+	{"adis16362", 1},
+	{"adis16364", 1},
+	{"adis16365", 1},
 	{}
 };
 
diff --git a/drivers/staging/iio/imu/adis16350_ring.c b/drivers/staging/iio/imu/adis16350_ring.c
index aefbae5..1970247 100644
--- a/drivers/staging/iio/imu/adis16350_ring.c
+++ b/drivers/staging/iio/imu/adis16350_ring.c
@@ -126,6 +126,56 @@ static int adis16350_spi_read_burst(struct device *dev, u8 *rx)
 	return ret;
 }
 
+static const u16 read_all_tx_array[] = {
+	be16_to_cpu(ADIS16350_READ_REG(ADIS16350_SUPPLY_OUT)),
+	be16_to_cpu(ADIS16350_READ_REG(ADIS16350_XGYRO_OUT)),
+	be16_to_cpu(ADIS16350_READ_REG(ADIS16350_YGYRO_OUT)),
+	be16_to_cpu(ADIS16350_READ_REG(ADIS16350_ZGYRO_OUT)),
+	be16_to_cpu(ADIS16350_READ_REG(ADIS16350_XACCL_OUT)),
+	be16_to_cpu(ADIS16350_READ_REG(ADIS16350_YACCL_OUT)),
+	be16_to_cpu(ADIS16350_READ_REG(ADIS16350_ZACCL_OUT)),
+	be16_to_cpu(ADIS16350_READ_REG(ADIS16350_XTEMP_OUT)),
+	be16_to_cpu(ADIS16350_READ_REG(ADIS16350_YTEMP_OUT)),
+	be16_to_cpu(ADIS16350_READ_REG(ADIS16350_ZTEMP_OUT)),
+	be16_to_cpu(ADIS16350_READ_REG(ADIS16350_AUX_ADC)),
+};
+
+static int adis16350_spi_read_all(struct device *dev, u16 *rx_array)
+{
+	struct iio_dev *indio_dev = dev_get_drvdata(dev);
+	struct adis16350_state *st = iio_dev_get_devdata(indio_dev);
+
+	struct spi_message msg;
+	int i, j = 0, ret;
+	struct spi_transfer *xfers;
+
+	xfers = kzalloc(sizeof(*xfers)*
+			st->indio_dev->ring->scan_count + 1,
+		GFP_KERNEL);
+	if (xfers == NULL)
+		return -ENOMEM;
+
+	for (i = 0; i < ARRAY_SIZE(read_all_tx_array); i++)
+		if (st->indio_dev->ring->scan_mask & (1 << i)) {
+			xfers[j].tx_buf = &read_all_tx_array[i];
+			xfers[j].bits_per_word = 16;
+			xfers[j].len = 2;
+			xfers[j + 1].rx_buf = rx_array + j;
+			j++;
+		}
+	xfers[j].bits_per_word = 16;
+	xfers[j].len = 2;
+
+	spi_message_init(&msg);
+	for (j = 0; j < st->indio_dev->ring->scan_count + 1; j++)
+		spi_message_add_tail(&xfers[j], &msg);
+
+	ret = spi_sync(st->us, &msg);
+	kfree(xfers);
+
+	return ret;
+}
+
 /* Whilst this makes a lot of calls to iio_sw_ring functions - it is to device
  * specific to be rolled into the core.
  */
@@ -136,22 +186,33 @@ static void adis16350_trigger_bh_to_ring(struct work_struct *work_s)
 			       work_trigger_to_ring);
 	struct iio_ring_buffer *ring = st->indio_dev->ring;
 
-	int i = 0;
+	int i = 0, j = 0;
 	s16 *data;
 	size_t datasize = ring->access.get_bytes_per_datum(ring);
 
-	data = kmalloc(datasize , GFP_KERNEL);
+	data = kmalloc(datasize, GFP_KERNEL);
 	if (data == NULL) {
 		dev_err(&st->us->dev, "memory alloc failed in ring bh");
 		return;
 	}
 
-	if (ring->scan_count)
-		if (adis16350_spi_read_burst(&st->indio_dev->dev, st->rx) >= 0)
+	if (ring->scan_count) {
+		if (st->burst_available) {
+			if (adis16350_spi_read_burst(&st->indio_dev->dev,
+							st->rx) < 0)
+				return;
+		for (; i < ring->scan_count; i++)
+			if (st->indio_dev->ring->scan_mask & (1 << i))
+				data[j++] = le16_to_cpup(
+					(__le16 *)&(st->rx[i*2]));
+		} else {
+			if (adis16350_spi_read_all(&st->indio_dev->dev,
+							(u16 *)st->rx) < 0)
+				return;
 			for (; i < ring->scan_count; i++)
-				data[i] = be16_to_cpup(
-					(__be16 *)&(st->rx[i*2]));
-
+				data[i] = *(u16 *)&(st->rx[i*2]);
+		}
+	}
 	/* Guaranteed to be aligned with 8 byte boundary */
 	if (ring->scan_timestamp)
 		*((s64 *)(data + ((i + 3)/4)*4)) = st->last_timestamp;
-- 
1.7.0.4

[PATCH 2/4] staging:iio:adis16350 move datardy trigger to straight interrupt.

[Jonathan Cameron]

The use of the interface for the trigger was lifted from the
lis3l02dq.  There it is needed because the same physical line
is used for data ready and event interrupts.  Her it isn't
so we can simplify things.

Signed-off-by: Jonathan Cameron <jic23@cam.ac.uk>
---
 drivers/staging/iio/imu/adis16350_core.c    |   27 ++------------
 drivers/staging/iio/imu/adis16350_trigger.c |   52 ++++++++-------------------
 2 files changed, 18 insertions(+), 61 deletions(-)

diff --git a/drivers/staging/iio/imu/adis16350_core.c b/drivers/staging/iio/imu/adis16350_core.c
index d8181f9..e500a5c 100644
--- a/drivers/staging/iio/imu/adis16350_core.c
+++ b/drivers/staging/iio/imu/adis16350_core.c
@@ -583,14 +583,6 @@ static const struct attribute_group adis16350_attribute_group = {
 	.attrs = adis16350_attributes,
 };
 
-static struct attribute *adis16350_event_attributes[] = {
-	NULL,
-};
-
-static struct attribute_group adis16350_event_attribute_group = {
-	.attrs = adis16350_event_attributes,
-};
-
 static int __devinit adis16350_probe(struct spi_device *spi)
 {
 	int ret, regdone = 0;
@@ -624,8 +616,6 @@ static int __devinit adis16350_probe(struct spi_device *spi)
 	}
 
 	st->indio_dev->dev.parent = &spi->dev;
-	st->indio_dev->num_interrupt_lines = 1;
-	st->indio_dev->event_attrs = &adis16350_event_attribute_group;
 	st->indio_dev->attrs = &adis16350_attribute_group;
 	st->indio_dev->dev_data = (void *)(st);
 	st->indio_dev->driver_module = THIS_MODULE;
@@ -647,17 +637,9 @@ static int __devinit adis16350_probe(struct spi_device *spi)
 	}
 
 	if (spi->irq) {
-		ret = iio_register_interrupt_line(spi->irq,
-				st->indio_dev,
-				0,
-				IRQF_TRIGGER_RISING,
-				"adis16350");
-		if (ret)
-			goto error_uninitialize_ring;
-
 		ret = adis16350_probe_trigger(st->indio_dev);
 		if (ret)
-			goto error_unregister_line;
+			goto error_uninitialize_ring;
 	}
 
 	/* Get the device into a sane initial state */
@@ -667,10 +649,8 @@ static int __devinit adis16350_probe(struct spi_device *spi)
 	return 0;
 
 error_remove_trigger:
-	adis16350_remove_trigger(st->indio_dev);
-error_unregister_line:
 	if (spi->irq)
-		iio_unregister_interrupt_line(st->indio_dev, 0);
+		adis16350_remove_trigger(st->indio_dev);
 error_uninitialize_ring:
 	iio_ring_buffer_unregister(st->indio_dev->ring);
 error_unreg_ring_funcs:
@@ -702,9 +682,8 @@ static int adis16350_remove(struct spi_device *spi)
 
 	flush_scheduled_work();
 
-	adis16350_remove_trigger(indio_dev);
 	if (spi->irq)
-		iio_unregister_interrupt_line(indio_dev, 0);
+		adis16350_remove_trigger(indio_dev);
 
 	iio_ring_buffer_unregister(indio_dev->ring);
 	iio_device_unregister(indio_dev);
diff --git a/drivers/staging/iio/imu/adis16350_trigger.c b/drivers/staging/iio/imu/adis16350_trigger.c
index 739b7ec..7f26de8 100644
--- a/drivers/staging/iio/imu/adis16350_trigger.c
+++ b/drivers/staging/iio/imu/adis16350_trigger.c
@@ -13,23 +13,14 @@
 #include "adis16350.h"
 
 /**
- * adis16350_data_rdy_trig_poll() the event handler for the data rdy trig
+ * adis16350_data_rdy_trig_poll() the interrupt th for the data rdy trig
  **/
-static int adis16350_data_rdy_trig_poll(struct iio_dev *dev_info,
-				       int index,
-				       s64 timestamp,
-				       int no_test)
+static irqreturn_t adis16350_data_rdy_trig_poll(int irq, void *private)
 {
-	struct adis16350_state *st = iio_dev_get_devdata(dev_info);
-	struct iio_trigger *trig = st->trig;
-
-	iio_trigger_poll(trig, timestamp);
-
+	iio_trigger_poll(private, iio_get_time_ns());
 	return IRQ_HANDLED;
 }
 
-IIO_EVENT_SH(data_rdy_trig, &adis16350_data_rdy_trig_poll);
-
 static IIO_TRIGGER_NAME_ATTR;
 
 static struct attribute *adis16350_trigger_attrs[] = {
@@ -53,36 +44,16 @@ static int adis16350_data_rdy_trigger_set_state(struct iio_trigger *trig,
 
 	dev_dbg(&indio_dev->dev, "%s (%d)\n", __func__, state);
 	ret = adis16350_set_irq(&st->indio_dev->dev, state);
-	if (state == false) {
-		iio_remove_event_from_list(&iio_event_data_rdy_trig,
-					   &indio_dev->interrupts[0]
-					   ->ev_list);
-		/* possible quirk with handler currently worked around
-		   by ensuring the work queue is empty */
+	if (state == false)
 		flush_scheduled_work();
-	} else {
-		iio_add_event_to_list(&iio_event_data_rdy_trig,
-				      &indio_dev->interrupts[0]->ev_list);
-	}
-	return ret;
-}
 
-/**
- * adis16350_trig_try_reen() try renabling irq for data rdy trigger
- * @trig:	the datardy trigger
- **/
-static int adis16350_trig_try_reen(struct iio_trigger *trig)
-{
-	struct adis16350_state *st = trig->private_data;
-	enable_irq(st->us->irq);
-	/* irq reenabled so success! */
-	return 0;
+	return ret;
 }
 
 int adis16350_probe_trigger(struct iio_dev *indio_dev)
 {
 	int ret;
-	struct adis16350_state *st = indio_dev->dev_data;
+	struct adis16350_state *st = iio_dev_get_devdata(indio_dev);
 
 	st->trig = iio_allocate_trigger();
 	st->trig->name = kasprintf(GFP_KERNEL,
@@ -96,7 +67,6 @@ int adis16350_probe_trigger(struct iio_dev *indio_dev)
 	st->trig->owner = THIS_MODULE;
 	st->trig->private_data = st;
 	st->trig->set_trigger_state = &adis16350_data_rdy_trigger_set_state;
-	st->trig->try_reenable = &adis16350_trig_try_reen;
 	st->trig->control_attrs = &adis16350_trigger_attr_group;
 	ret = iio_trigger_register(st->trig);
 
@@ -105,8 +75,15 @@ int adis16350_probe_trigger(struct iio_dev *indio_dev)
 	if (ret)
 		goto error_free_trig_name;
 
+	ret = request_irq(st->us->irq, adis16350_data_rdy_trig_poll,
+			IRQF_TRIGGER_RISING, st->trig->name, st->trig);
+	if (ret)
+		goto error_trigger_unregister;
+
 	return 0;
 
+error_trigger_unregister:
+	iio_trigger_unregister(st->trig);
 error_free_trig_name:
 	kfree(st->trig->name);
 error_free_trig:
@@ -117,8 +94,9 @@ error_free_trig:
 
 void adis16350_remove_trigger(struct iio_dev *indio_dev)
 {
-	struct adis16350_state *state = indio_dev->dev_data;
+	struct adis16350_state *state = iio_dev_get_devdata(indio_dev);
 
+	free_irq(state->us->irq, state->trig);
 	iio_trigger_unregister(state->trig);
 	kfree(state->trig->name);
 	iio_free_trigger(state->trig);
-- 
1.7.0.4

[PATCH 3/4] staging:iio:adis16350 Add optional event support

[Jonathan Cameron]

This is fairly complex.  The device provides two alarms
which can be configured as rising or falling thresholds
on the magnitude or the rate of change of any channel.

I have made things a little more manageable by restricting
things so you can't set both alarms to the same setting.

This also rolls in some name changes etc to make possible
use of macros for some elements.

Note there is an issue with event codes for ADC channels (there
are now too many of them) that will need fixing before this merges.

Various other minor fixes are also in here.
---
 drivers/staging/iio/adc/adc.h            |    8 +++-
 drivers/staging/iio/gyro/gyro.h          |   31 ++++++++++----
 drivers/staging/iio/imu/Kconfig          |    7 +++
 drivers/staging/iio/imu/Makefile         |    1 +
 drivers/staging/iio/imu/adis16350.h      |   62 ++++++++++++++++++++++++-----
 drivers/staging/iio/imu/adis16350_core.c |   17 ++++++--
 drivers/staging/iio/imu/adis16350_ring.c |   10 ++--
 drivers/staging/iio/sysfs.h              |   17 ++++++++-
 8 files changed, 122 insertions(+), 31 deletions(-)

diff --git a/drivers/staging/iio/adc/adc.h b/drivers/staging/iio/adc/adc.h
index 953b5ce..2cd2ef9 100644
--- a/drivers/staging/iio/adc/adc.h
+++ b/drivers/staging/iio/adc/adc.h
@@ -11,7 +11,7 @@
 
 /* Deprecated */
 #define IIO_DEV_ATTR_ADC(_num, _show, _addr)			\
-  IIO_DEVICE_ATTR(adc_##_num, S_IRUGO, _show, NULL, _addr)
+	IIO_DEVICE_ATTR(adc_##_num, S_IRUGO, _show, NULL, _addr)
 
 #define IIO_DEV_ATTR_IN_RAW(_num, _show, _addr)				\
 	IIO_DEVICE_ATTR(in##_num##_raw, S_IRUGO, _show, NULL, _addr)
@@ -36,3 +36,9 @@
 
 #define IIO_EVENT_CODE_IN_HIGH_THRESH(a) (IIO_EVENT_CODE_ADC_BASE  + a)
 #define IIO_EVENT_CODE_IN_LOW_THRESH(a) (IIO_EVENT_CODE_ADC_BASE  + a + 32)
+#define IIO_EVENT_CODE_IN_HIGH_ROC(a) (IIO_EVENT_CODE_ADC_BASE  + a + 64)
+#define IIO_EVENT_CODE_IN_LOW_ROC(a) (IIO_EVENT_CODE_ADC_BASE  + a + 96)
+#define IIO_EVENT_CODE_IN_SUPPLY_HIGH (IIO_EVENT_CODE_ADC_BASE + 97)
+#define IIO_EVENT_CODE_IN_SUPPLY_LOW (IIO_EVENT_CODE_ADC_BASE + 98)
+#define IIO_EVENT_CODE_IN_SUPPLY_ROC_HIGH (IIO_EVENT_CODE_ADC_BASE + 99)
+#define IIO_EVENT_CODE_IN_SUPPLY_ROC_LOW (IIO_EVENT_CODE_ADC_BASE + 100)
diff --git a/drivers/staging/iio/gyro/gyro.h b/drivers/staging/iio/gyro/gyro.h
index 98b837b..7cda511 100644
--- a/drivers/staging/iio/gyro/gyro.h
+++ b/drivers/staging/iio/gyro/gyro.h
@@ -33,31 +33,31 @@
 #define IIO_DEV_ATTR_GYRO_Z_SCALE(_mode, _show, _store, _addr)		\
 	IIO_DEVICE_ATTR(gyro_z_scale, _mode, _show, _store, _addr)
 
-#define IIO_DEV_ATTR_GYRO_CALIBBIAS(_mode, _show, _store, _addr)		\
+#define IIO_DEV_ATTR_GYRO_CALIBBIAS(_mode, _show, _store, _addr)	\
 	IIO_DEVICE_ATTR(gyro_calibbias, S_IRUGO, _show, _store, _addr)
 
-#define IIO_DEV_ATTR_GYRO_X_CALIBBIAS(_mode, _show, _store, _addr)		\
+#define IIO_DEV_ATTR_GYRO_X_CALIBBIAS(_mode, _show, _store, _addr)	\
 	IIO_DEVICE_ATTR(gyro_x_calibbias, _mode, _show, _store, _addr)
 
-#define IIO_DEV_ATTR_GYRO_Y_CALIBBIAS(_mode, _show, _store, _addr)		\
+#define IIO_DEV_ATTR_GYRO_Y_CALIBBIAS(_mode, _show, _store, _addr)	\
 	IIO_DEVICE_ATTR(gyro_y_calibbias, _mode, _show, _store, _addr)
 
-#define IIO_DEV_ATTR_GYRO_Z_CALIBBIAS(_mode, _show, _store, _addr)		\
+#define IIO_DEV_ATTR_GYRO_Z_CALIBBIAS(_mode, _show, _store, _addr)	\
 	IIO_DEVICE_ATTR(gyro_z_calibbias, _mode, _show, _store, _addr)
 
-#define IIO_DEV_ATTR_GYRO_CALIBSCALE(_mode, _show, _store, _addr)		\
+#define IIO_DEV_ATTR_GYRO_CALIBSCALE(_mode, _show, _store, _addr)	\
 	IIO_DEVICE_ATTR(gyro_calibscale, S_IRUGO, _show, _store, _addr)
 
-#define IIO_DEV_ATTR_GYRO_X_CALIBSCALE(_mode, _show, _store, _addr)		\
+#define IIO_DEV_ATTR_GYRO_X_CALIBSCALE(_mode, _show, _store, _addr)	\
 	IIO_DEVICE_ATTR(gyro_x_calibscale, _mode, _show, _store, _addr)
 
-#define IIO_DEV_ATTR_GYRO_Y_CALIBSCALE(_mode, _show, _store, _addr)		\
+#define IIO_DEV_ATTR_GYRO_Y_CALIBSCALE(_mode, _show, _store, _addr)	\
 	IIO_DEVICE_ATTR(gyro_y_calibscale, _mode, _show, _store, _addr)
 
-#define IIO_DEV_ATTR_GYRO_Z_CALIBSCALE(_mode, _show, _store, _addr)		\
+#define IIO_DEV_ATTR_GYRO_Z_CALIBSCALE(_mode, _show, _store, _addr)	\
 	IIO_DEVICE_ATTR(gyro_z_calibscale, _mode, _show, _store, _addr)
 
-#define IIO_DEV_ATTR_GYRO(_show, _addr)			\
+#define IIO_DEV_ATTR_GYRO(_show, _addr)				\
 	IIO_DEVICE_ATTR(gyro_raw, S_IRUGO, _show, NULL, _addr)
 
 #define IIO_DEV_ATTR_GYRO_X(_show, _addr)			\
@@ -71,3 +71,16 @@
 
 #define IIO_DEV_ATTR_ANGL(_show, _addr)                         \
 	IIO_DEVICE_ATTR(angl_raw, S_IRUGO, _show, NULL, _addr)
+
+#define IIO_EVENT_CODE_GYRO_X_HIGH (IIO_EVENT_CODE_GYRO_BASE)
+#define IIO_EVENT_CODE_GYRO_X_LOW (IIO_EVENT_CODE_GYRO_BASE + 1)
+#define IIO_EVENT_CODE_GYRO_X_ROC_HIGH (IIO_EVENT_CODE_GYRO_BASE + 2)
+#define IIO_EVENT_CODE_GYRO_X_ROC_LOW (IIO_EVENT_CODE_GYRO_BASE + 3)
+#define IIO_EVENT_CODE_GYRO_Y_HIGH (IIO_EVENT_CODE_GYRO_BASE + 4)
+#define IIO_EVENT_CODE_GYRO_Y_LOW (IIO_EVENT_CODE_GYRO_BASE + 5)
+#define IIO_EVENT_CODE_GYRO_Y_ROC_HIGH (IIO_EVENT_CODE_GYRO_BASE + 6)
+#define IIO_EVENT_CODE_GYRO_Y_ROC_LOW (IIO_EVENT_CODE_GYRO_BASE + 7)
+#define IIO_EVENT_CODE_GYRO_Z_HIGH (IIO_EVENT_CODE_GYRO_BASE+ 8)
+#define IIO_EVENT_CODE_GYRO_Z_LOW (IIO_EVENT_CODE_GYRO_BASE + 9)
+#define IIO_EVENT_CODE_GYRO_Z_ROC_HIGH (IIO_EVENT_CODE_GYRO_BASE + 10)
+#define IIO_EVENT_CODE_GYRO_Z_ROC_LOW (IIO_EVENT_CODE_GYRO_BASE + 11)
diff --git a/drivers/staging/iio/imu/Kconfig b/drivers/staging/iio/imu/Kconfig
index 31a6233..11a4984 100644
--- a/drivers/staging/iio/imu/Kconfig
+++ b/drivers/staging/iio/imu/Kconfig
@@ -21,6 +21,13 @@ config ADIS16350
 	  Say yes here to build support for Analog Devices adis16350/54/55/60/62/64/65
 	  high precision tri-axis inertial sensor.
 
+if ADIS16350
+config ADIS16350_EVENT
+       bool "Enable events on the ADIS16350"
+       help
+	 Threshold and rate of change events for all channels.
+endif #ADIS16350
+
 config ADIS16400
 	tristate "Analog Devices ADIS16400/5 IMU SPI driver"
 	depends on SPI
diff --git a/drivers/staging/iio/imu/Makefile b/drivers/staging/iio/imu/Makefile
index f3b450b..551a20f 100644
--- a/drivers/staging/iio/imu/Makefile
+++ b/drivers/staging/iio/imu/Makefile
@@ -7,6 +7,7 @@ adis16300-$(CONFIG_IIO_RING_BUFFER) += adis16300_ring.o adis16300_trigger.o
 obj-$(CONFIG_ADIS16300) += adis16300.o
 
 adis16350-y             := adis16350_core.o
+adis16350-$(CONFIG_ADIS16350_EVENT) += adis16350_event.o
 adis16350-$(CONFIG_IIO_RING_BUFFER) += adis16350_ring.o adis16350_trigger.o
 obj-$(CONFIG_ADIS16350) += adis16350.o
 
diff --git a/drivers/staging/iio/imu/adis16350.h b/drivers/staging/iio/imu/adis16350.h
index 07d3eeb..948fe75 100644
--- a/drivers/staging/iio/imu/adis16350.h
+++ b/drivers/staging/iio/imu/adis16350.h
@@ -108,6 +108,13 @@
  * @rx:			recieve buffer
  * @buf_lock:		mutex to protect tx and rx
  * @burst_available:	does the device support burst reading
+ * @event_lock:		protect event state
+ * @event_irq:		irq for event line
+ * @event_timestamp:	local store for time of last event
+ * @work_event:		work struct for event bh
+ * @thresh_cache:	cache of thresholds for all the various alarms
+ * @smpl_cache:		cache of sample period for ROC alarms
+ * @active_alarms:	which alarms are on (in terms of attribute mask)
  **/
 struct adis16350_state {
 	struct spi_device		*us;
@@ -119,23 +126,56 @@ struct adis16350_state {
 	u8				*rx;
 	struct mutex			buf_lock;
 	unsigned int			burst_available:1;
+#ifdef CONFIG_ADIS16350_EVENT
+	struct mutex			event_lock;
+	int				event_irq;
+	s64				event_timestamp;
+	struct work_struct		work_event;
+	s16				thresh_cache[44];
+	u8				smpl_cache[22];
+	int				active_alarms[2];
+	unsigned int			oldest_alarm:1;
+	u8				alarm_on[2];
+#endif
 };
 
 int adis16350_set_irq(struct device *dev, bool enable);
 
+int adis16350_spi_read_reg_16(struct device *dev,
+			u8 lower_reg_address,
+			u16 *val);
+
+int adis16350_spi_write_reg_16(struct device *dev,
+			u8 lower_reg_address,
+			u16 value);
+
+#ifdef CONFIG_ADIS16350_EVENT
+int adis16350_configure_event_line(struct adis16350_state *st,
+				int interrupt);
+void adis16350_unconfigure_event_line(struct adis16350_state *st);
+#else
+static inline int adis16350_configure_event_line(struct adis16350_state *st,
+						int interrupt)
+{
+	return 0;
+}
+static inline void adis16350_unconfigure_event_line(struct adis16350_state *st)
+{
+}
+#endif
 #ifdef CONFIG_IIO_RING_BUFFER
 
-#define ADIS16350_SCAN_SUPPLY	0
-#define ADIS16350_SCAN_GYRO_X	1
-#define ADIS16350_SCAN_GYRO_Y	2
-#define ADIS16350_SCAN_GYRO_Z	3
-#define ADIS16350_SCAN_ACC_X	4
-#define ADIS16350_SCAN_ACC_Y	5
-#define ADIS16350_SCAN_ACC_Z	6
-#define ADIS16350_SCAN_TEMP_X	7
-#define ADIS16350_SCAN_TEMP_Y	8
-#define ADIS16350_SCAN_TEMP_Z	9
-#define ADIS16350_SCAN_ADC_0	10
+#define ADIS16350_SCAN_IN_SUPPLY	0
+#define ADIS16350_SCAN_GYRO_X		1
+#define ADIS16350_SCAN_GYRO_Y		2
+#define ADIS16350_SCAN_GYRO_Z		3
+#define ADIS16350_SCAN_ACCEL_X		4
+#define ADIS16350_SCAN_ACCEL_Y		5
+#define ADIS16350_SCAN_ACCEL_Z		6
+#define ADIS16350_SCAN_TEMP_X		7
+#define ADIS16350_SCAN_TEMP_Y		8
+#define ADIS16350_SCAN_TEMP_Z		9
+#define ADIS16350_SCAN_IN0		10
 
 void adis16350_remove_trigger(struct iio_dev *indio_dev);
 int adis16350_probe_trigger(struct iio_dev *indio_dev);
diff --git a/drivers/staging/iio/imu/adis16350_core.c b/drivers/staging/iio/imu/adis16350_core.c
index e500a5c..8480dc2 100644
--- a/drivers/staging/iio/imu/adis16350_core.c
+++ b/drivers/staging/iio/imu/adis16350_core.c
@@ -62,7 +62,7 @@ static int adis16350_spi_write_reg_8(struct device *dev,
  *               is assumed to have address one greater.
  * @val: value to be written
  **/
-static int adis16350_spi_write_reg_16(struct device *dev,
+int adis16350_spi_write_reg_16(struct device *dev,
 		u8 lower_reg_address,
 		u16 value)
 {
@@ -108,7 +108,7 @@ static int adis16350_spi_write_reg_16(struct device *dev,
  *               is assumed to have address one greater.
  * @val: somewhere to pass back the value read
  **/
-static int adis16350_spi_read_reg_16(struct device *dev,
+int adis16350_spi_read_reg_16(struct device *dev,
 		u8 lower_reg_address,
 		u16 *val)
 {
@@ -641,13 +641,22 @@ static int __devinit adis16350_probe(struct spi_device *spi)
 		if (ret)
 			goto error_uninitialize_ring;
 	}
-
+	/* The event irq */
+	if (spi->dev.platform_data) {
+		ret = adis16350_configure_event_line(st,
+						*(int *)spi->dev.platform_data);
+ 		if (ret)
+			goto error_remove_trigger;
+ 	}
 	/* Get the device into a sane initial state */
 	ret = adis16350_initial_setup(st);
 	if (ret)
-		goto error_remove_trigger;
+		goto error_unregister_event_line;
+
 	return 0;
 
+error_unregister_event_line:
+	adis16350_unconfigure_event_line(st);
 error_remove_trigger:
 	if (spi->irq)
 		adis16350_remove_trigger(st->indio_dev);
diff --git a/drivers/staging/iio/imu/adis16350_ring.c b/drivers/staging/iio/imu/adis16350_ring.c
index 1970247..9af0345 100644
--- a/drivers/staging/iio/imu/adis16350_ring.c
+++ b/drivers/staging/iio/imu/adis16350_ring.c
@@ -17,7 +17,7 @@
 #include "../trigger.h"
 #include "adis16350.h"
 
-static IIO_SCAN_EL_C(in_supply, ADIS16350_SCAN_SUPPLY, IIO_UNSIGNED(12),
+static IIO_SCAN_EL_C(in_supply, ADIS16350_SCAN_IN_SUPPLY, IIO_UNSIGNED(12),
 		ADIS16350_SUPPLY_OUT, NULL);
 
 static IIO_SCAN_EL_C(gyro_x, ADIS16350_SCAN_GYRO_X, IIO_SIGNED(14),
@@ -27,11 +27,11 @@ static IIO_SCAN_EL_C(gyro_y, ADIS16350_SCAN_GYRO_Y, IIO_SIGNED(14),
 static IIO_SCAN_EL_C(gyro_z, ADIS16350_SCAN_GYRO_Z, IIO_SIGNED(14),
 		ADIS16350_ZGYRO_OUT, NULL);
 
-static IIO_SCAN_EL_C(accel_x, ADIS16350_SCAN_ACC_X, IIO_SIGNED(14),
+static IIO_SCAN_EL_C(accel_x, ADIS16350_SCAN_ACCEL_X, IIO_SIGNED(14),
 		ADIS16350_XACCL_OUT, NULL);
-static IIO_SCAN_EL_C(accel_y, ADIS16350_SCAN_ACC_Y, IIO_SIGNED(14),
+static IIO_SCAN_EL_C(accel_y, ADIS16350_SCAN_ACCEL_Y, IIO_SIGNED(14),
 		ADIS16350_YACCL_OUT, NULL);
-static IIO_SCAN_EL_C(accel_z, ADIS16350_SCAN_ACC_Z, IIO_SIGNED(14),
+static IIO_SCAN_EL_C(accel_z, ADIS16350_SCAN_ACCEL_Z, IIO_SIGNED(14),
 		ADIS16350_ZACCL_OUT, NULL);
 
 static IIO_SCAN_EL_C(temp_x, ADIS16350_SCAN_TEMP_X, IIO_SIGNED(12),
@@ -41,7 +41,7 @@ static IIO_SCAN_EL_C(temp_y, ADIS16350_SCAN_TEMP_Y, IIO_SIGNED(12),
 static IIO_SCAN_EL_C(temp_z, ADIS16350_SCAN_TEMP_Z, IIO_SIGNED(12),
 		ADIS16350_ZTEMP_OUT, NULL);
 
-static IIO_SCAN_EL_C(in0, ADIS16350_SCAN_ADC_0, IIO_UNSIGNED(12),
+static IIO_SCAN_EL_C(in0, ADIS16350_SCAN_IN0, IIO_UNSIGNED(12),
 		ADIS16350_AUX_ADC, NULL);
 
 static IIO_SCAN_EL_TIMESTAMP(11);
diff --git a/drivers/staging/iio/sysfs.h b/drivers/staging/iio/sysfs.h
index b531dc1..bd3edd4 100644
--- a/drivers/staging/iio/sysfs.h
+++ b/drivers/staging/iio/sysfs.h
@@ -63,7 +63,7 @@ struct iio_const_attr {
 	container_of(_dev_attr, struct iio_const_attr, dev_attr)
 
 /* Some attributes will be hard coded (device dependent) and not require an
-   address, in these cases pass a negative */
+ * address, in these cases pass a negative */
 #define IIO_ATTR(_name, _mode, _show, _store, _addr)		\
 	{ .dev_attr = __ATTR(_name, _mode, _show, _store),	\
 	  .address = _addr }
@@ -256,8 +256,23 @@ struct iio_const_attr {
 #define IIO_EVENT_CODE_ADC_BASE		500
 #define IIO_EVENT_CODE_MISC_BASE	600
 #define IIO_EVENT_CODE_LIGHT_BASE	700
+#define IIO_EVENT_CODE_TEMP_BASE	800
 
 #define IIO_EVENT_CODE_DEVICE_SPECIFIC	1000
+/* Can we make these a little more predictable rather than
+ * just adding new ones when they turn up? */
+#define IIO_EVENT_CODE_TEMP_X_HIGH (IIO_EVENT_CODE_TEMP_BASE)
+#define IIO_EVENT_CODE_TEMP_X_LOW (IIO_EVENT_CODE_TEMP_BASE + 1)
+#define IIO_EVENT_CODE_TEMP_X_ROC_HIGH (IIO_EVENT_CODE_TEMP_BASE + 2)
+#define IIO_EVENT_CODE_TEMP_X_ROC_LOW (IIO_EVENT_CODE_TEMP_BASE + 3)
+#define IIO_EVENT_CODE_TEMP_Y_HIGH (IIO_EVENT_CODE_TEMP_BASE + 4)
+#define IIO_EVENT_CODE_TEMP_Y_LOW (IIO_EVENT_CODE_TEMP_BASE + 5)
+#define IIO_EVENT_CODE_TEMP_Y_ROC_HIGH (IIO_EVENT_CODE_TEMP_BASE + 6)
+#define IIO_EVENT_CODE_TEMP_Y_ROC_LOW (IIO_EVENT_CODE_TEMP_BASE + 7)
+#define IIO_EVENT_CODE_TEMP_Z_HIGH (IIO_EVENT_CODE_TEMP_BASE + 8)
+#define IIO_EVENT_CODE_TEMP_Z_LOW (IIO_EVENT_CODE_TEMP_BASE + 9)
+#define IIO_EVENT_CODE_TEMP_Z_ROC_HIGH (IIO_EVENT_CODE_TEMP_BASE + 10)
+#define IIO_EVENT_CODE_TEMP_Z_ROC_LOW (IIO_EVENT_CODE_TEMP_BASE + 11)
 
 /**
  * IIO_EVENT_ATTR_RING_50_FULL - ring buffer event to indicate 50% full
-- 
1.7.0.4

[PATCH 4/4] staging:iio:adis16350 add missing registration of temp_offset attr

[Jonathan Cameron]

Signed-off-by: Jonathan Cameron <jic23@cam.ac.uk>
---
 drivers/staging/iio/imu/adis16350_core.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/staging/iio/imu/adis16350_core.c b/drivers/staging/iio/imu/adis16350_core.c
index 8480dc2..b344cd9 100644
--- a/drivers/staging/iio/imu/adis16350_core.c
+++ b/drivers/staging/iio/imu/adis16350_core.c
@@ -570,6 +570,7 @@ static struct attribute *adis16350_attributes[] = {
 	&iio_dev_attr_temp_y_raw.dev_attr.attr,
 	&iio_dev_attr_temp_z_raw.dev_attr.attr,
 	&iio_const_attr_temp_scale.dev_attr.attr,
+	&iio_const_attr_temp_offset.dev_attr.attr,
 	&iio_dev_attr_in0_raw.dev_attr.attr,
 	&iio_const_attr_in0_scale.dev_attr.attr,
 	&iio_dev_attr_sampling_frequency.dev_attr.attr,
-- 
1.7.0.4

Re: [PATCH 2/4] staging:iio:adis16350 move datardy trigger to straight interrupt.

[Manuel Stahl]

[-- Attachment #1: Type: text/plain, Size: 7030 bytes --]

Am 06.09.2010 00:20, schrieb Jonathan Cameron:
> The use of the interface for the trigger was lifted from the
> lis3l02dq.  There it is needed because the same physical line
> is used for data ready and event interrupts.  Her it isn't
> so we can simplify things.

I guess this patch has to be applied to all other adis devices as well.

>
> Signed-off-by: Jonathan Cameron<jic23@cam.ac.uk>
> ---
>   drivers/staging/iio/imu/adis16350_core.c    |   27 ++------------
>   drivers/staging/iio/imu/adis16350_trigger.c |   52 ++++++++-------------------
>   2 files changed, 18 insertions(+), 61 deletions(-)
>
> diff --git a/drivers/staging/iio/imu/adis16350_core.c b/drivers/staging/iio/imu/adis16350_core.c
> index d8181f9..e500a5c 100644
> --- a/drivers/staging/iio/imu/adis16350_core.c
> +++ b/drivers/staging/iio/imu/adis16350_core.c
> @@ -583,14 +583,6 @@ static const struct attribute_group adis16350_attribute_group = {
>   	.attrs = adis16350_attributes,
>   };
>
> -static struct attribute *adis16350_event_attributes[] = {
> -	NULL,
> -};
> -
> -static struct attribute_group adis16350_event_attribute_group = {
> -	.attrs = adis16350_event_attributes,
> -};
> -
>   static int __devinit adis16350_probe(struct spi_device *spi)
>   {
>   	int ret, regdone = 0;
> @@ -624,8 +616,6 @@ static int __devinit adis16350_probe(struct spi_device *spi)
>   	}
>
>   	st->indio_dev->dev.parent =&spi->dev;
> -	st->indio_dev->num_interrupt_lines = 1;
> -	st->indio_dev->event_attrs =&adis16350_event_attribute_group;
>   	st->indio_dev->attrs =&adis16350_attribute_group;
>   	st->indio_dev->dev_data = (void *)(st);
>   	st->indio_dev->driver_module = THIS_MODULE;
> @@ -647,17 +637,9 @@ static int __devinit adis16350_probe(struct spi_device *spi)
>   	}
>
>   	if (spi->irq) {
> -		ret = iio_register_interrupt_line(spi->irq,
> -				st->indio_dev,
> -				0,
> -				IRQF_TRIGGER_RISING,
> -				"adis16350");
> -		if (ret)
> -			goto error_uninitialize_ring;
> -
>   		ret = adis16350_probe_trigger(st->indio_dev);
>   		if (ret)
> -			goto error_unregister_line;
> +			goto error_uninitialize_ring;
>   	}
>
>   	/* Get the device into a sane initial state */
> @@ -667,10 +649,8 @@ static int __devinit adis16350_probe(struct spi_device *spi)
>   	return 0;
>
>   error_remove_trigger:
> -	adis16350_remove_trigger(st->indio_dev);
> -error_unregister_line:
>   	if (spi->irq)
> -		iio_unregister_interrupt_line(st->indio_dev, 0);
> +		adis16350_remove_trigger(st->indio_dev);
>   error_uninitialize_ring:
>   	iio_ring_buffer_unregister(st->indio_dev->ring);
>   error_unreg_ring_funcs:
> @@ -702,9 +682,8 @@ static int adis16350_remove(struct spi_device *spi)
>
>   	flush_scheduled_work();
>
> -	adis16350_remove_trigger(indio_dev);
>   	if (spi->irq)
> -		iio_unregister_interrupt_line(indio_dev, 0);
> +		adis16350_remove_trigger(indio_dev);
>
>   	iio_ring_buffer_unregister(indio_dev->ring);
>   	iio_device_unregister(indio_dev);
> diff --git a/drivers/staging/iio/imu/adis16350_trigger.c b/drivers/staging/iio/imu/adis16350_trigger.c
> index 739b7ec..7f26de8 100644
> --- a/drivers/staging/iio/imu/adis16350_trigger.c
> +++ b/drivers/staging/iio/imu/adis16350_trigger.c
> @@ -13,23 +13,14 @@
>   #include "adis16350.h"
>
>   /**
> - * adis16350_data_rdy_trig_poll() the event handler for the data rdy trig
> + * adis16350_data_rdy_trig_poll() the interrupt th for the data rdy trig
>    **/
> -static int adis16350_data_rdy_trig_poll(struct iio_dev *dev_info,
> -				       int index,
> -				       s64 timestamp,
> -				       int no_test)
> +static irqreturn_t adis16350_data_rdy_trig_poll(int irq, void *private)
>   {
> -	struct adis16350_state *st = iio_dev_get_devdata(dev_info);
> -	struct iio_trigger *trig = st->trig;
> -
> -	iio_trigger_poll(trig, timestamp);
> -
> +	iio_trigger_poll(private, iio_get_time_ns());
>   	return IRQ_HANDLED;
>   }
>
> -IIO_EVENT_SH(data_rdy_trig,&adis16350_data_rdy_trig_poll);
> -
>   static IIO_TRIGGER_NAME_ATTR;
>
>   static struct attribute *adis16350_trigger_attrs[] = {
> @@ -53,36 +44,16 @@ static int adis16350_data_rdy_trigger_set_state(struct iio_trigger *trig,
>
>   	dev_dbg(&indio_dev->dev, "%s (%d)\n", __func__, state);
>   	ret = adis16350_set_irq(&st->indio_dev->dev, state);
> -	if (state == false) {
> -		iio_remove_event_from_list(&iio_event_data_rdy_trig,
> -					&indio_dev->interrupts[0]
> -					   ->ev_list);
> -		/* possible quirk with handler currently worked around
> -		   by ensuring the work queue is empty */
> +	if (state == false)
>   		flush_scheduled_work();
> -	} else {
> -		iio_add_event_to_list(&iio_event_data_rdy_trig,
> -				&indio_dev->interrupts[0]->ev_list);
> -	}
> -	return ret;
> -}
>
> -/**
> - * adis16350_trig_try_reen() try renabling irq for data rdy trigger
> - * @trig:	the datardy trigger
> - **/
> -static int adis16350_trig_try_reen(struct iio_trigger *trig)
> -{
> -	struct adis16350_state *st = trig->private_data;
> -	enable_irq(st->us->irq);
> -	/* irq reenabled so success! */
> -	return 0;
> +	return ret;
>   }
>
>   int adis16350_probe_trigger(struct iio_dev *indio_dev)
>   {
>   	int ret;
> -	struct adis16350_state *st = indio_dev->dev_data;
> +	struct adis16350_state *st = iio_dev_get_devdata(indio_dev);
>
>   	st->trig = iio_allocate_trigger();
>   	st->trig->name = kasprintf(GFP_KERNEL,
> @@ -96,7 +67,6 @@ int adis16350_probe_trigger(struct iio_dev *indio_dev)
>   	st->trig->owner = THIS_MODULE;
>   	st->trig->private_data = st;
>   	st->trig->set_trigger_state =&adis16350_data_rdy_trigger_set_state;
> -	st->trig->try_reenable =&adis16350_trig_try_reen;
>   	st->trig->control_attrs =&adis16350_trigger_attr_group;
>   	ret = iio_trigger_register(st->trig);
>
> @@ -105,8 +75,15 @@ int adis16350_probe_trigger(struct iio_dev *indio_dev)
>   	if (ret)
>   		goto error_free_trig_name;
>
> +	ret = request_irq(st->us->irq, adis16350_data_rdy_trig_poll,
> +			IRQF_TRIGGER_RISING, st->trig->name, st->trig);
> +	if (ret)
> +		goto error_trigger_unregister;
> +
>   	return 0;
>
> +error_trigger_unregister:
> +	iio_trigger_unregister(st->trig);
>   error_free_trig_name:
>   	kfree(st->trig->name);
>   error_free_trig:
> @@ -117,8 +94,9 @@ error_free_trig:
>
>   void adis16350_remove_trigger(struct iio_dev *indio_dev)
>   {
> -	struct adis16350_state *state = indio_dev->dev_data;
> +	struct adis16350_state *state = iio_dev_get_devdata(indio_dev);
>
> +	free_irq(state->us->irq, state->trig);
>   	iio_trigger_unregister(state->trig);
>   	kfree(state->trig->name);
>   	iio_free_trigger(state->trig);


-- 
Dipl.-Inf. Manuel Stahl
Fraunhofer-Institut für Integrierte Schaltungen IIS
- Leistungsoptimierte Systeme -
Nordostpark 93                Telefon  +49 (0)911/58061-6419
90411 Nürnberg                Fax      +49 (0)911/58061-6398
http://www.iis.fraunhofer.de  manuel.stahl@iis.fraunhofer.de

[-- Attachment #2: manuel_stahl.vcf --]
[-- Type: text/x-vcard, Size: 161 bytes --]

begin:vcard
fn:Manuel Stahl
n:Stahl;Manuel
email;internet:manuel.stahl@iis.fraunhofer.de
tel;work:+49 911 58061-6419
x-mozilla-html:FALSE
version:2.1
end:vcard

Re: [RFC PATCH 0/4] staging:iio:adis16350 various updates and fixes

[Manuel Stahl]

[-- Attachment #1.1: Type: text/plain, Size: 1846 bytes --]

Am 06.09.2010 00:20, schrieb Jonathan Cameron:
>
> Patch 3 puts in event support.  It's complex, but then what the
> device has some complex abilities. Still if anyone can see any
> simplifications without breaking the interface I would definitely
> like to hear them!

> @@ -36,3 +36,9 @@
>
>  #define IIO_EVENT_CODE_IN_HIGH_THRESH(a) (IIO_EVENT_CODE_ADC_BASE  + a)
>  #define IIO_EVENT_CODE_IN_LOW_THRESH(a) (IIO_EVENT_CODE_ADC_BASE  + a + 32)
> +#define IIO_EVENT_CODE_IN_HIGH_ROC(a) (IIO_EVENT_CODE_ADC_BASE  + a + 64)
> +#define IIO_EVENT_CODE_IN_LOW_ROC(a) (IIO_EVENT_CODE_ADC_BASE  + a + 96)
> +#define IIO_EVENT_CODE_IN_SUPPLY_HIGH (IIO_EVENT_CODE_ADC_BASE + 97)
> +#define IIO_EVENT_CODE_IN_SUPPLY_LOW (IIO_EVENT_CODE_ADC_BASE + 98)
> +#define IIO_EVENT_CODE_IN_SUPPLY_ROC_HIGH (IIO_EVENT_CODE_ADC_BASE + 99)
> +#define IIO_EVENT_CODE_IN_SUPPLY_ROC_LOW (IIO_EVENT_CODE_ADC_BASE + 100)

What about putting some more structure in the event codes.
Maybe <type_id> <channel> <event_id> each in it's own byte, where
<type_id> is one of accel, gyro, ...
<channel> is one of x, y, z
<event_id> is one of high_thresh, low_thresh, ...
maybe even a defined bit for high or low, one for thresh or roc, ...

Then we don't need a big event table, but can figure out all parts 
independently.

Could we generate some of the event attributes in sysfs on the fly? Like 
have an attribute alarm0_input, where we can write in a list of channels 
and then only generate the limit attributes for these channels.

Regards,
-- 
Dipl.-Inf. Manuel Stahl
Fraunhofer-Institut für Integrierte Schaltungen IIS
- Leistungsoptimierte Systeme -
Nordostpark 93                Telefon  +49 (0)911/58061-6419
90411 Nürnberg                Fax      +49 (0)911/58061-6398
http://www.iis.fraunhofer.de  manuel.stahl@iis.fraunhofer.de

[-- Attachment #1.2: manuel_stahl.vcf --]
[-- Type: text/x-vcard, Size: 170 bytes --]

begin:vcard
fn:Manuel Stahl
n:Stahl;Manuel
email;internet:manuel.stahl@iis.fraunhofer.de
tel;work:+49 911 58061-6419
x-mozilla-html:FALSE
version:2.1
end:vcard


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 6148 bytes --]

Re: [RFC PATCH 0/4] staging:iio:adis16350 various updates and fixes

[Jonathan Cameron]

On 09/06/10 10:00, Manuel Stahl wrote:
> Am 06.09.2010 00:20, schrieb Jonathan Cameron:
>>
>> Patch 3 puts in event support.  It's complex, but then what the
>> device has some complex abilities. Still if anyone can see any
>> simplifications without breaking the interface I would definitely
>> like to hear them!
> 
>> @@ -36,3 +36,9 @@
>>
>>  #define IIO_EVENT_CODE_IN_HIGH_THRESH(a) (IIO_EVENT_CODE_ADC_BASE  + a)
>>  #define IIO_EVENT_CODE_IN_LOW_THRESH(a) (IIO_EVENT_CODE_ADC_BASE  + a + 32)
>> +#define IIO_EVENT_CODE_IN_HIGH_ROC(a) (IIO_EVENT_CODE_ADC_BASE  + a + 64)
>> +#define IIO_EVENT_CODE_IN_LOW_ROC(a) (IIO_EVENT_CODE_ADC_BASE  + a + 96)
>> +#define IIO_EVENT_CODE_IN_SUPPLY_HIGH (IIO_EVENT_CODE_ADC_BASE + 97)
>> +#define IIO_EVENT_CODE_IN_SUPPLY_LOW (IIO_EVENT_CODE_ADC_BASE + 98)
>> +#define IIO_EVENT_CODE_IN_SUPPLY_ROC_HIGH (IIO_EVENT_CODE_ADC_BASE + 99)
>> +#define IIO_EVENT_CODE_IN_SUPPLY_ROC_LOW (IIO_EVENT_CODE_ADC_BASE + 100)
> 
> What about putting some more structure in the event codes.
> Maybe <type_id> <channel> <event_id> each in it's own byte, where
> <type_id> is one of accel, gyro, ...
> <channel> is one of x, y, z
> <event_id> is one of high_thresh, low_thresh, ...
> maybe even a defined bit for high or low, one for thresh or roc, ...
> 
> Then we don't need a big event table, but can figure out all parts independently.
Good idea.  Lets merge this driver and any others in this round as we have it
then propose a change to something more organized that we can then apply to all in
tree drivers at the same time.  This won't effect Analog's drivers much as they
haven't made extensive use of the event interface as yet.

Perhaps we want to open this one up to more general opinions (lkml)?  It's a big
change and we always said we would keep posting them there.
> 
> Could we generate some of the event attributes in sysfs on the fly?
> Like have an attribute alarm0_input, where we can write in a list of
> channels and then only generate the limit attributes for these
> channels.
Not terribly generalizable.  We basically have the same issue we had with
devices with limited scan options (max1363). We have about half of devices
with alarms on every channel and half like this one that have a small finite number
of alarms that we can connect to a given device.  I really don't want to go
towards having two different interfaces for the two cases (like we had for
a while with the scan mode stuff).  The cost here is lots of attributes,
but I don't think it is too bad and I doubt we'll ever have any devices
that are significantly worse than this one... With the macro usage it doesn't
lead to all that much code.

Jonathan

Re: [PATCH 2/4] staging:iio:adis16350 move datardy trigger to straight interrupt.

[Jonathan Cameron]

On 09/06/10 09:13, Manuel Stahl wrote:
> Am 06.09.2010 00:20, schrieb Jonathan Cameron:
>> The use of the interface for the trigger was lifted from the
>> lis3l02dq.  There it is needed because the same physical line
>> is used for data ready and event interrupts.  Her it isn't
>> so we can simplify things.
> 
> I guess this patch has to be applied to all other adis devices as well.
Agreed, that was on my todo list ;)
> 
>>
>> Signed-off-by: Jonathan Cameron<jic23@cam.ac.uk>
>> ---
>>   drivers/staging/iio/imu/adis16350_core.c    |   27 ++------------
>>   drivers/staging/iio/imu/adis16350_trigger.c |   52 ++++++++-------------------
>>   2 files changed, 18 insertions(+), 61 deletions(-)
>>
>> diff --git a/drivers/staging/iio/imu/adis16350_core.c b/drivers/staging/iio/imu/adis16350_core.c
>> index d8181f9..e500a5c 100644
>> --- a/drivers/staging/iio/imu/adis16350_core.c
>> +++ b/drivers/staging/iio/imu/adis16350_core.c
>> @@ -583,14 +583,6 @@ static const struct attribute_group adis16350_attribute_group = {
>>       .attrs = adis16350_attributes,
>>   };
>>
>> -static struct attribute *adis16350_event_attributes[] = {
>> -    NULL,
>> -};
>> -
>> -static struct attribute_group adis16350_event_attribute_group = {
>> -    .attrs = adis16350_event_attributes,
>> -};
>> -
>>   static int __devinit adis16350_probe(struct spi_device *spi)
>>   {
>>       int ret, regdone = 0;
>> @@ -624,8 +616,6 @@ static int __devinit adis16350_probe(struct spi_device *spi)
>>       }
>>
>>       st->indio_dev->dev.parent =&spi->dev;
>> -    st->indio_dev->num_interrupt_lines = 1;
>> -    st->indio_dev->event_attrs =&adis16350_event_attribute_group;
>>       st->indio_dev->attrs =&adis16350_attribute_group;
>>       st->indio_dev->dev_data = (void *)(st);
>>       st->indio_dev->driver_module = THIS_MODULE;
>> @@ -647,17 +637,9 @@ static int __devinit adis16350_probe(struct spi_device *spi)
>>       }
>>
>>       if (spi->irq) {
>> -        ret = iio_register_interrupt_line(spi->irq,
>> -                st->indio_dev,
>> -                0,
>> -                IRQF_TRIGGER_RISING,
>> -                "adis16350");
>> -        if (ret)
>> -            goto error_uninitialize_ring;
>> -
>>           ret = adis16350_probe_trigger(st->indio_dev);
>>           if (ret)
>> -            goto error_unregister_line;
>> +            goto error_uninitialize_ring;
>>       }
>>
>>       /* Get the device into a sane initial state */
>> @@ -667,10 +649,8 @@ static int __devinit adis16350_probe(struct spi_device *spi)
>>       return 0;
>>
>>   error_remove_trigger:
>> -    adis16350_remove_trigger(st->indio_dev);
>> -error_unregister_line:
>>       if (spi->irq)
>> -        iio_unregister_interrupt_line(st->indio_dev, 0);
>> +        adis16350_remove_trigger(st->indio_dev);
>>   error_uninitialize_ring:
>>       iio_ring_buffer_unregister(st->indio_dev->ring);
>>   error_unreg_ring_funcs:
>> @@ -702,9 +682,8 @@ static int adis16350_remove(struct spi_device *spi)
>>
>>       flush_scheduled_work();
>>
>> -    adis16350_remove_trigger(indio_dev);
>>       if (spi->irq)
>> -        iio_unregister_interrupt_line(indio_dev, 0);
>> +        adis16350_remove_trigger(indio_dev);
>>
>>       iio_ring_buffer_unregister(indio_dev->ring);
>>       iio_device_unregister(indio_dev);
>> diff --git a/drivers/staging/iio/imu/adis16350_trigger.c b/drivers/staging/iio/imu/adis16350_trigger.c
>> index 739b7ec..7f26de8 100644
>> --- a/drivers/staging/iio/imu/adis16350_trigger.c
>> +++ b/drivers/staging/iio/imu/adis16350_trigger.c
>> @@ -13,23 +13,14 @@
>>   #include "adis16350.h"
>>
>>   /**
>> - * adis16350_data_rdy_trig_poll() the event handler for the data rdy trig
>> + * adis16350_data_rdy_trig_poll() the interrupt th for the data rdy trig
>>    **/
>> -static int adis16350_data_rdy_trig_poll(struct iio_dev *dev_info,
>> -                       int index,
>> -                       s64 timestamp,
>> -                       int no_test)
>> +static irqreturn_t adis16350_data_rdy_trig_poll(int irq, void *private)
>>   {
>> -    struct adis16350_state *st = iio_dev_get_devdata(dev_info);
>> -    struct iio_trigger *trig = st->trig;
>> -
>> -    iio_trigger_poll(trig, timestamp);
>> -
>> +    iio_trigger_poll(private, iio_get_time_ns());
>>       return IRQ_HANDLED;
>>   }
>>
>> -IIO_EVENT_SH(data_rdy_trig,&adis16350_data_rdy_trig_poll);
>> -
>>   static IIO_TRIGGER_NAME_ATTR;
>>
>>   static struct attribute *adis16350_trigger_attrs[] = {
>> @@ -53,36 +44,16 @@ static int adis16350_data_rdy_trigger_set_state(struct iio_trigger *trig,
>>
>>       dev_dbg(&indio_dev->dev, "%s (%d)\n", __func__, state);
>>       ret = adis16350_set_irq(&st->indio_dev->dev, state);
>> -    if (state == false) {
>> -        iio_remove_event_from_list(&iio_event_data_rdy_trig,
>> -                    &indio_dev->interrupts[0]
>> -                       ->ev_list);
>> -        /* possible quirk with handler currently worked around
>> -           by ensuring the work queue is empty */
>> +    if (state == false)
>>           flush_scheduled_work();
>> -    } else {
>> -        iio_add_event_to_list(&iio_event_data_rdy_trig,
>> -                &indio_dev->interrupts[0]->ev_list);
>> -    }
>> -    return ret;
>> -}
>>
>> -/**
>> - * adis16350_trig_try_reen() try renabling irq for data rdy trigger
>> - * @trig:    the datardy trigger
>> - **/
>> -static int adis16350_trig_try_reen(struct iio_trigger *trig)
>> -{
>> -    struct adis16350_state *st = trig->private_data;
>> -    enable_irq(st->us->irq);
>> -    /* irq reenabled so success! */
>> -    return 0;
>> +    return ret;
>>   }
>>
>>   int adis16350_probe_trigger(struct iio_dev *indio_dev)
>>   {
>>       int ret;
>> -    struct adis16350_state *st = indio_dev->dev_data;
>> +    struct adis16350_state *st = iio_dev_get_devdata(indio_dev);
>>
>>       st->trig = iio_allocate_trigger();
>>       st->trig->name = kasprintf(GFP_KERNEL,
>> @@ -96,7 +67,6 @@ int adis16350_probe_trigger(struct iio_dev *indio_dev)
>>       st->trig->owner = THIS_MODULE;
>>       st->trig->private_data = st;
>>       st->trig->set_trigger_state =&adis16350_data_rdy_trigger_set_state;
>> -    st->trig->try_reenable =&adis16350_trig_try_reen;
>>       st->trig->control_attrs =&adis16350_trigger_attr_group;
>>       ret = iio_trigger_register(st->trig);
>>
>> @@ -105,8 +75,15 @@ int adis16350_probe_trigger(struct iio_dev *indio_dev)
>>       if (ret)
>>           goto error_free_trig_name;
>>
>> +    ret = request_irq(st->us->irq, adis16350_data_rdy_trig_poll,
>> +            IRQF_TRIGGER_RISING, st->trig->name, st->trig);
>> +    if (ret)
>> +        goto error_trigger_unregister;
>> +
>>       return 0;
>>
>> +error_trigger_unregister:
>> +    iio_trigger_unregister(st->trig);
>>   error_free_trig_name:
>>       kfree(st->trig->name);
>>   error_free_trig:
>> @@ -117,8 +94,9 @@ error_free_trig:
>>
>>   void adis16350_remove_trigger(struct iio_dev *indio_dev)
>>   {
>> -    struct adis16350_state *state = indio_dev->dev_data;
>> +    struct adis16350_state *state = iio_dev_get_devdata(indio_dev);
>>
>> +    free_irq(state->us->irq, state->trig);
>>       iio_trigger_unregister(state->trig);
>>       kfree(state->trig->name);
>>       iio_free_trigger(state->trig);
> 
> 

Re: [PATCH 3/4] staging:iio:adis16350 Add optional event support

[Jonathan Cameron]

On 09/05/10 23:20, Jonathan Cameron wrote:
> This is fairly complex.  The device provides two alarms
> which can be configured as rising or falling thresholds
> on the magnitude or the rate of change of any channel.
Oops.  This patch doesn't actually contain adis16350_event.c
that is the actual event code. Sorry about that.

Jonathan
> 
> I have made things a little more manageable by restricting
> things so you can't set both alarms to the same setting.
> 
> This also rolls in some name changes etc to make possible
> use of macros for some elements.
> 
> Note there is an issue with event codes for ADC channels (there
> are now too many of them) that will need fixing before this merges.
> 
> Various other minor fixes are also in here.
> ---
>  drivers/staging/iio/adc/adc.h            |    8 +++-
>  drivers/staging/iio/gyro/gyro.h          |   31 ++++++++++----
>  drivers/staging/iio/imu/Kconfig          |    7 +++
>  drivers/staging/iio/imu/Makefile         |    1 +
>  drivers/staging/iio/imu/adis16350.h      |   62 ++++++++++++++++++++++++-----
>  drivers/staging/iio/imu/adis16350_core.c |   17 ++++++--
>  drivers/staging/iio/imu/adis16350_ring.c |   10 ++--
>  drivers/staging/iio/sysfs.h              |   17 ++++++++-
>  8 files changed, 122 insertions(+), 31 deletions(-)