[RFC PATCH 0/2] IIO: Filtering - how to handle.

[RFC PATCH 0/2] IIO: Filtering - how to handle.

[Jonathan Cameron]

Hi All,

One big area we have pretty much glossed over so far is devices with
controllable hardware filters.  This RFC proposes one option for
how to handle this. For low pass filters at least, the 3db point
seems the obvious choice as it allows us to gloss over exactly
what type of filter it is whilst still capturing it's basic
property of what it lets through.

What do people think?

Jonathan

Jonathan Cameron (2):
  staging:iio: filter description - low pass 3db frequency.
  staging:iio:imu:adis16400 add control of data filtering.

 drivers/staging/iio/iio.h                |    2 +
 drivers/staging/iio/imu/adis16400.h      |    2 +
 drivers/staging/iio/imu/adis16400_core.c |  177 +++++++++++++++++++++++------
 drivers/staging/iio/industrialio-core.c  |    2 +
 4 files changed, 146 insertions(+), 37 deletions(-)

-- 
1.7.3.4

[PATCH 1/2] staging:iio: filter description - low pass 3db frequency.

[Jonathan Cameron]

Introduce the first data filtering related parameter.
For now we are ignoring the filter type and merely specifying its
approximate (I read them off tiny graphs) 3db point.

Signed-off-by: Jonathan Cameron <jic23@cam.ac.uk>
---
 drivers/staging/iio/iio.h               |    2 ++
 drivers/staging/iio/industrialio-core.c |    2 ++
 2 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/drivers/staging/iio/iio.h b/drivers/staging/iio/iio.h
index 2a6c673..3c39e1c 100644
--- a/drivers/staging/iio/iio.h
+++ b/drivers/staging/iio/iio.h
@@ -79,6 +79,8 @@ enum iio_chan_info_enum {
 	IIO_CHAN_INFO_QUADRATURE_CORRECTION_RAW_SEPARATE,
 	IIO_CHAN_INFO_AVERAGE_RAW_SHARED,
 	IIO_CHAN_INFO_AVERAGE_RAW_SEPARATE,
+	IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY_SHARED,
+	IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY_SEPARATE,
 };
 
 enum iio_endian {
diff --git a/drivers/staging/iio/industrialio-core.c b/drivers/staging/iio/industrialio-core.c
index 647a405..d2af50a 100644
--- a/drivers/staging/iio/industrialio-core.c
+++ b/drivers/staging/iio/industrialio-core.c
@@ -86,6 +86,8 @@ static const char * const iio_chan_info_postfix[] = {
 	[IIO_CHAN_INFO_QUADRATURE_CORRECTION_RAW_SHARED/2]
 	= "quadrature_correction_raw",
 	[IIO_CHAN_INFO_AVERAGE_RAW_SHARED/2] = "mean_raw",
+	[IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY_SHARED/2]
+	= "filter_low_pass_3db_frequency",
 };
 
 /**
-- 
1.7.3.4

[PATCH 2/2] staging:iio:imu:adis16400 add control of data filtering.

[Jonathan Cameron]

Bartlett filter used.  Values read of figures on datasheet
so far from precise.

Signed-off-by: Jonathan Cameron <jic23@cam.ac.uk>
---
 drivers/staging/iio/imu/adis16400.h      |    2 +
 drivers/staging/iio/imu/adis16400_core.c |  177 +++++++++++++++++++++++------
 2 files changed, 142 insertions(+), 37 deletions(-)

diff --git a/drivers/staging/iio/imu/adis16400.h b/drivers/staging/iio/imu/adis16400.h
index f3546ee..83d133e 100644
--- a/drivers/staging/iio/imu/adis16400.h
+++ b/drivers/staging/iio/imu/adis16400.h
@@ -148,12 +148,14 @@ struct adis16400_chip_info {
  * @tx:			transmit buffer
  * @rx:			receive buffer
  * @buf_lock:		mutex to protect tx and rx
+ * @filt_int:		integer part of requested filter frequency
  **/
 struct adis16400_state {
 	struct spi_device		*us;
 	struct iio_trigger		*trig;
 	struct mutex			buf_lock;
 	struct adis16400_chip_info	*variant;
+	int				filt_int;
 
 	u8	tx[ADIS16400_MAX_TX] ____cacheline_aligned;
 	u8	rx[ADIS16400_MAX_RX] ____cacheline_aligned;
diff --git a/drivers/staging/iio/imu/adis16400_core.c b/drivers/staging/iio/imu/adis16400_core.c
index 07d5a61..fe9b6a5 100644
--- a/drivers/staging/iio/imu/adis16400_core.c
+++ b/drivers/staging/iio/imu/adis16400_core.c
@@ -161,25 +161,65 @@ error_ret:
 	return ret;
 }
 
+static int adis16400_get_freq(struct iio_dev *indio_dev)
+{
+	u16 t;
+	int sps, ret;
+
+	ret = adis16400_spi_read_reg_16(indio_dev, ADIS16400_SMPL_PRD, &t);
+	if (ret < 0)
+		return ret;
+	sps =  (t & ADIS16400_SMPL_PRD_TIME_BASE) ? 53 : 1638;
+	sps /= (t & ADIS16400_SMPL_PRD_DIV_MASK) + 1;
+
+	return sps;
+}
+
 static ssize_t adis16400_read_frequency(struct device *dev,
 		struct device_attribute *attr,
 		char *buf)
 {
 	struct iio_dev *indio_dev = dev_get_drvdata(dev);
 	int ret, len = 0;
-	u16 t;
-	int sps;
-	ret = adis16400_spi_read_reg_16(indio_dev,
-			ADIS16400_SMPL_PRD,
-			&t);
-	if (ret)
+	ret = adis16400_get_freq(indio_dev);
+	if (ret < 0)
 		return ret;
-	sps =  (t & ADIS16400_SMPL_PRD_TIME_BASE) ? 53 : 1638;
-	sps /= (t & ADIS16400_SMPL_PRD_DIV_MASK) + 1;
-	len = sprintf(buf, "%d SPS\n", sps);
+	len = sprintf(buf, "%d SPS\n", ret);
 	return len;
 }
 
+static const unsigned adis16400_3db_divisors[] = {
+	[0] = 2, /* Special case */
+	[1] = 5,
+	[2] = 10,
+	[3] = 50,
+	[4] = 200,
+};
+
+static int adis16400_set_filter(struct iio_dev *indio_dev, int sps, int val)
+{
+	int i, ret;
+	u16 val16;
+	for (i = ARRAY_SIZE(adis16400_3db_divisors) - 1; i >= 0; i--)
+		if (sps/adis16400_3db_divisors[i] > val)
+			break;
+	if (i == -1)
+		ret = -EINVAL;
+	else {
+		ret = adis16400_spi_read_reg_16(indio_dev,
+						ADIS16400_SENS_AVG,
+						&val16);
+		if (ret < 0)
+			goto error_ret;
+
+		ret = adis16400_spi_write_reg_16(indio_dev,
+						 ADIS16400_SENS_AVG,
+						 (val16 & ~0x03) | i);
+	}
+error_ret:
+	return ret;
+}
+
 static ssize_t adis16400_write_frequency(struct device *dev,
 		struct device_attribute *attr,
 		const char *buf,
@@ -210,6 +250,7 @@ static ssize_t adis16400_write_frequency(struct device *dev,
 			ADIS16400_SMPL_PRD,
 			t);
 
+	/* Also update the filter */
 	mutex_unlock(&indio_dev->mlock);
 
 	return ret ? ret : len;
@@ -455,13 +496,15 @@ static u8 adis16400_addresses[17][2] = {
 	[incli_y] = { ADIS16300_ROLL_OUT }
 };
 
+
 static int adis16400_write_raw(struct iio_dev *indio_dev,
 			       struct iio_chan_spec const *chan,
 			       int val,
 			       int val2,
 			       long mask)
 {
-	int ret;
+	struct adis16400_state *st = iio_priv(indio_dev);
+	int ret, sps;
 
 	switch (mask) {
 	case (1 << IIO_CHAN_INFO_CALIBBIAS_SEPARATE):
@@ -471,6 +514,21 @@ static int adis16400_write_raw(struct iio_dev *indio_dev,
 				val);
 		mutex_unlock(&indio_dev->mlock);
 		return ret;
+	case (1 << IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY_SHARED):
+		/* Need to cache values so we can update if the frequency
+		   changes */
+		mutex_lock(&indio_dev->mlock);
+		st->filt_int = val;
+		/* Work out update to current value */
+		sps = adis16400_get_freq(indio_dev);
+		if (sps < 0) {
+			mutex_unlock(&indio_dev->mlock);
+			return sps;
+		}
+
+		ret = adis16400_set_filter(indio_dev, sps, val);
+		mutex_unlock(&indio_dev->mlock);
+		return ret;
 	default:
 		return -EINVAL;
 	}
@@ -549,6 +607,24 @@ static int adis16400_read_raw(struct iio_dev *indio_dev,
 		*val = 198;
 		*val2 = 160000;
 		return IIO_VAL_INT_PLUS_MICRO;
+	case (1 << IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY_SHARED):
+		mutex_lock(&indio_dev->mlock);
+		/* Need both the number of taps and the sampling frequency */
+		ret = adis16400_spi_read_reg_16(indio_dev,
+						ADIS16400_SENS_AVG,
+						&val16);
+		if (ret < 0) {
+			mutex_unlock(&indio_dev->mlock);
+			return ret;
+		}
+		ret = adis16400_get_freq(indio_dev);
+		if (ret > 0)
+			*val = ret/adis16400_3db_divisors[val16 & 0x03];
+		*val2 = 0;
+		mutex_unlock(&indio_dev->mlock);
+		if (ret < 0)
+			return ret;
+		return IIO_VAL_INT_PLUS_MICRO;
 	default:
 		return -EINVAL;
 	}
@@ -569,7 +645,8 @@ static struct iio_chan_spec adis16400_channels[] = {
 		.modified = 1,
 		.channel2 = IIO_MOD_X,
 		.info_mask = (1 << IIO_CHAN_INFO_CALIBBIAS_SEPARATE) |
-		(1 << IIO_CHAN_INFO_SCALE_SHARED),
+		(1 << IIO_CHAN_INFO_SCALE_SHARED) |
+		(1 << IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY_SHARED),
 		.address = gyro_x,
 		.scan_index = ADIS16400_SCAN_GYRO_X,
 		.scan_type = IIO_ST('s', 14, 16, 0)
@@ -578,7 +655,8 @@ static struct iio_chan_spec adis16400_channels[] = {
 		.modified = 1,
 		.channel2 = IIO_MOD_Y,
 		.info_mask = (1 << IIO_CHAN_INFO_CALIBBIAS_SEPARATE) |
-		(1 << IIO_CHAN_INFO_SCALE_SHARED),
+		(1 << IIO_CHAN_INFO_SCALE_SHARED) |
+		(1 << IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY_SHARED),
 		.address = gyro_y,
 		.scan_index = ADIS16400_SCAN_GYRO_Y,
 		.scan_type = IIO_ST('s', 14, 16, 0),
@@ -587,7 +665,8 @@ static struct iio_chan_spec adis16400_channels[] = {
 		.modified = 1,
 		.channel2 = IIO_MOD_Z,
 		.info_mask = (1 << IIO_CHAN_INFO_CALIBBIAS_SEPARATE) |
-		(1 << IIO_CHAN_INFO_SCALE_SHARED),
+		(1 << IIO_CHAN_INFO_SCALE_SHARED) |
+		(1 << IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY_SHARED),
 		.address = gyro_z,
 		.scan_index = ADIS16400_SCAN_GYRO_Z,
 		.scan_type = IIO_ST('s', 14, 16, 0),
@@ -596,7 +675,8 @@ static struct iio_chan_spec adis16400_channels[] = {
 		.modified = 1,
 		.channel2 = IIO_MOD_X,
 		.info_mask = (1 << IIO_CHAN_INFO_CALIBBIAS_SEPARATE) |
-		(1 << IIO_CHAN_INFO_SCALE_SHARED),
+		(1 << IIO_CHAN_INFO_SCALE_SHARED) |
+		(1 << IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY_SHARED),
 		.address = accel_x,
 		.scan_index = ADIS16400_SCAN_ACC_X,
 		.scan_type = IIO_ST('s', 14, 16, 0),
@@ -605,7 +685,8 @@ static struct iio_chan_spec adis16400_channels[] = {
 		.modified = 1,
 		.channel2 = IIO_MOD_Y,
 		.info_mask = (1 << IIO_CHAN_INFO_CALIBBIAS_SEPARATE) |
-		(1 << IIO_CHAN_INFO_SCALE_SHARED),
+		(1 << IIO_CHAN_INFO_SCALE_SHARED) |
+		(1 << IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY_SHARED),
 		.address = accel_y,
 		.scan_index = ADIS16400_SCAN_ACC_Y,
 		.scan_type = IIO_ST('s', 14, 16, 0),
@@ -614,7 +695,8 @@ static struct iio_chan_spec adis16400_channels[] = {
 		.modified = 1,
 		.channel2 = IIO_MOD_Z,
 		.info_mask = (1 << IIO_CHAN_INFO_CALIBBIAS_SEPARATE) |
-		(1 << IIO_CHAN_INFO_SCALE_SHARED),
+		(1 << IIO_CHAN_INFO_SCALE_SHARED) |
+		(1 << IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY_SHARED),
 		.address = accel_z,
 		.scan_index = ADIS16400_SCAN_ACC_Z,
 		.scan_type = IIO_ST('s', 14, 16, 0),
@@ -622,7 +704,8 @@ static struct iio_chan_spec adis16400_channels[] = {
 		.type = IIO_MAGN,
 		.modified = 1,
 		.channel2 = IIO_MOD_X,
-		.info_mask = (1 << IIO_CHAN_INFO_SCALE_SHARED),
+		.info_mask = (1 << IIO_CHAN_INFO_SCALE_SHARED) |
+		(1 << IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY_SHARED),
 		.address = magn_x,
 		.scan_index = ADIS16400_SCAN_MAGN_X,
 		.scan_type = IIO_ST('s', 14, 16, 0),
@@ -630,7 +713,8 @@ static struct iio_chan_spec adis16400_channels[] = {
 		.type = IIO_MAGN,
 		.modified = 1,
 		.channel2 = IIO_MOD_Y,
-		.info_mask = (1 << IIO_CHAN_INFO_SCALE_SHARED),
+		.info_mask = (1 << IIO_CHAN_INFO_SCALE_SHARED) |
+		(1 << IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY_SHARED),
 		.address = magn_y,
 		.scan_index = ADIS16400_SCAN_MAGN_Y,
 		.scan_type = IIO_ST('s', 14, 16, 0),
@@ -638,7 +722,8 @@ static struct iio_chan_spec adis16400_channels[] = {
 		.type = IIO_MAGN,
 		.modified = 1,
 		.channel2 = IIO_MOD_Z,
-		.info_mask = (1 << IIO_CHAN_INFO_SCALE_SHARED),
+		.info_mask = (1 << IIO_CHAN_INFO_SCALE_SHARED) |
+		(1 << IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY_SHARED),
 		.address = magn_z,
 		.scan_index = ADIS16400_SCAN_MAGN_Z,
 		.scan_type = IIO_ST('s', 14, 16, 0),
@@ -678,7 +763,8 @@ static struct iio_chan_spec adis16350_channels[] = {
 		.modified = 1,
 		.channel2 = IIO_MOD_X,
 		.info_mask = (1 << IIO_CHAN_INFO_CALIBBIAS_SEPARATE) |
-		(1 << IIO_CHAN_INFO_SCALE_SHARED),
+		(1 << IIO_CHAN_INFO_SCALE_SHARED) |
+		(1 << IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY_SHARED),
 		.address = gyro_x,
 		.scan_index = ADIS16400_SCAN_GYRO_X,
 		.scan_type = IIO_ST('s', 14, 16, 0)
@@ -687,7 +773,8 @@ static struct iio_chan_spec adis16350_channels[] = {
 		.modified = 1,
 		.channel2 = IIO_MOD_Y,
 		.info_mask = (1 << IIO_CHAN_INFO_CALIBBIAS_SEPARATE) |
-		(1 << IIO_CHAN_INFO_SCALE_SHARED),
+		(1 << IIO_CHAN_INFO_SCALE_SHARED) |
+		(1 << IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY_SHARED),
 		.address = gyro_y,
 		.scan_index = ADIS16400_SCAN_GYRO_Y,
 		.scan_type = IIO_ST('s', 14, 16, 0),
@@ -696,7 +783,8 @@ static struct iio_chan_spec adis16350_channels[] = {
 		.modified = 1,
 		.channel2 = IIO_MOD_Z,
 		.info_mask = (1 << IIO_CHAN_INFO_CALIBBIAS_SEPARATE) |
-		(1 << IIO_CHAN_INFO_SCALE_SHARED),
+		(1 << IIO_CHAN_INFO_SCALE_SHARED) |
+		(1 << IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY_SHARED),
 		.address = gyro_z,
 		.scan_index = ADIS16400_SCAN_GYRO_Z,
 		.scan_type = IIO_ST('s', 14, 16, 0),
@@ -705,7 +793,8 @@ static struct iio_chan_spec adis16350_channels[] = {
 		.modified = 1,
 		.channel2 = IIO_MOD_X,
 		.info_mask = (1 << IIO_CHAN_INFO_CALIBBIAS_SEPARATE) |
-		(1 << IIO_CHAN_INFO_SCALE_SHARED),
+		(1 << IIO_CHAN_INFO_SCALE_SHARED) |
+		(1 << IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY_SHARED),
 		.address = accel_x,
 		.scan_index = ADIS16400_SCAN_ACC_X,
 		.scan_type = IIO_ST('s', 14, 16, 0),
@@ -714,7 +803,8 @@ static struct iio_chan_spec adis16350_channels[] = {
 		.modified = 1,
 		.channel2 = IIO_MOD_Y,
 		.info_mask = (1 << IIO_CHAN_INFO_CALIBBIAS_SEPARATE) |
-		(1 << IIO_CHAN_INFO_SCALE_SHARED),
+		(1 << IIO_CHAN_INFO_SCALE_SHARED) |
+		(1 << IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY_SHARED),
 		.address = accel_y,
 		.scan_index = ADIS16400_SCAN_ACC_Y,
 		.scan_type = IIO_ST('s', 14, 16, 0),
@@ -723,7 +813,8 @@ static struct iio_chan_spec adis16350_channels[] = {
 		.modified = 1,
 		.channel2 = IIO_MOD_Z,
 		.info_mask = (1 << IIO_CHAN_INFO_CALIBBIAS_SEPARATE) |
-		(1 << IIO_CHAN_INFO_SCALE_SHARED),
+		(1 << IIO_CHAN_INFO_SCALE_SHARED) |
+		(1 << IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY_SHARED),
 		.address = accel_z,
 		.scan_index = ADIS16400_SCAN_ACC_Z,
 		.scan_type = IIO_ST('s', 14, 16, 0),
@@ -733,7 +824,8 @@ static struct iio_chan_spec adis16350_channels[] = {
 		.channel = 0,
 		.extend_name = "x",
 		.info_mask = (1 << IIO_CHAN_INFO_OFFSET_SEPARATE) |
-		(1 << IIO_CHAN_INFO_SCALE_SEPARATE),
+		(1 << IIO_CHAN_INFO_SCALE_SEPARATE) |
+		(1 << IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY_SHARED),
 		.address = temp0,
 		.scan_index = ADIS16350_SCAN_TEMP_X,
 		.scan_type = IIO_ST('s', 12, 16, 0),
@@ -743,7 +835,8 @@ static struct iio_chan_spec adis16350_channels[] = {
 		.channel = 1,
 		.extend_name = "y",
 		.info_mask = (1 << IIO_CHAN_INFO_OFFSET_SEPARATE) |
-		(1 << IIO_CHAN_INFO_SCALE_SEPARATE),
+		(1 << IIO_CHAN_INFO_SCALE_SEPARATE) |
+		(1 << IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY_SHARED),
 		.address = temp1,
 		.scan_index = ADIS16350_SCAN_TEMP_Y,
 		.scan_type = IIO_ST('s', 12, 16, 0),
@@ -784,7 +877,8 @@ static struct iio_chan_spec adis16300_channels[] = {
 		.modified = 1,
 		.channel2 = IIO_MOD_X,
 		.info_mask = (1 << IIO_CHAN_INFO_CALIBBIAS_SEPARATE) |
-		(1 << IIO_CHAN_INFO_SCALE_SHARED),
+		(1 << IIO_CHAN_INFO_SCALE_SHARED) |
+		(1 << IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY_SHARED),
 		.address = gyro_x,
 		.scan_index = ADIS16400_SCAN_GYRO_X,
 		.scan_type = IIO_ST('s', 14, 16, 0),
@@ -793,7 +887,8 @@ static struct iio_chan_spec adis16300_channels[] = {
 		.modified = 1,
 		.channel2 = IIO_MOD_X,
 		.info_mask = (1 << IIO_CHAN_INFO_CALIBBIAS_SEPARATE) |
-		(1 << IIO_CHAN_INFO_SCALE_SHARED),
+		(1 << IIO_CHAN_INFO_SCALE_SHARED) |
+		(1 << IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY_SHARED),
 		.address = accel_x,
 		.scan_index = ADIS16400_SCAN_ACC_X,
 		.scan_type = IIO_ST('s', 14, 16, 0),
@@ -802,7 +897,8 @@ static struct iio_chan_spec adis16300_channels[] = {
 		.modified = 1,
 		.channel2 = IIO_MOD_Y,
 		.info_mask = (1 << IIO_CHAN_INFO_CALIBBIAS_SEPARATE) |
-		(1 << IIO_CHAN_INFO_SCALE_SHARED),
+		(1 << IIO_CHAN_INFO_SCALE_SHARED) |
+		(1 << IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY_SHARED),
 		.address = accel_y,
 		.scan_index = ADIS16400_SCAN_ACC_Y,
 		.scan_type = IIO_ST('s', 14, 16, 0),
@@ -811,7 +907,8 @@ static struct iio_chan_spec adis16300_channels[] = {
 		.modified = 1,
 		.channel2 = IIO_MOD_Z,
 		.info_mask = (1 << IIO_CHAN_INFO_CALIBBIAS_SEPARATE) |
-		(1 << IIO_CHAN_INFO_SCALE_SHARED),
+		(1 << IIO_CHAN_INFO_SCALE_SHARED) |
+		(1 << IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY_SHARED),
 		.address = accel_z,
 		.scan_index = ADIS16400_SCAN_ACC_Z,
 		.scan_type = IIO_ST('s', 14, 16, 0),
@@ -858,7 +955,8 @@ static const struct iio_chan_spec adis16334_channels[] = {
 		.modified = 1,
 		.channel2 = IIO_MOD_X,
 		.info_mask = (1 << IIO_CHAN_INFO_CALIBBIAS_SEPARATE) |
-		(1 << IIO_CHAN_INFO_SCALE_SHARED),
+		(1 << IIO_CHAN_INFO_SCALE_SHARED) |
+		(1 << IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY_SHARED),
 		.address = gyro_x,
 		.scan_index = ADIS16400_SCAN_GYRO_X,
 		.scan_type = IIO_ST('s', 14, 16, 0),
@@ -867,7 +965,8 @@ static const struct iio_chan_spec adis16334_channels[] = {
 		.modified = 1,
 		.channel2 = IIO_MOD_Y,
 		.info_mask = (1 << IIO_CHAN_INFO_CALIBBIAS_SEPARATE) |
-		(1 << IIO_CHAN_INFO_SCALE_SHARED),
+		(1 << IIO_CHAN_INFO_SCALE_SHARED) |
+		(1 << IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY_SHARED),
 		.address = gyro_y,
 		.scan_index = ADIS16400_SCAN_GYRO_Y,
 		.scan_type = IIO_ST('s', 14, 16, 0),
@@ -876,7 +975,8 @@ static const struct iio_chan_spec adis16334_channels[] = {
 		.modified = 1,
 		.channel2 = IIO_MOD_Z,
 		.info_mask = (1 << IIO_CHAN_INFO_CALIBBIAS_SEPARATE) |
-		(1 << IIO_CHAN_INFO_SCALE_SHARED),
+		(1 << IIO_CHAN_INFO_SCALE_SHARED) |
+		(1 << IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY_SHARED),
 		.address = gyro_z,
 		.scan_index = ADIS16400_SCAN_GYRO_Z,
 		.scan_type = IIO_ST('s', 14, 16, 0),
@@ -885,7 +985,8 @@ static const struct iio_chan_spec adis16334_channels[] = {
 		.modified = 1,
 		.channel2 = IIO_MOD_X,
 		.info_mask = (1 << IIO_CHAN_INFO_CALIBBIAS_SEPARATE) |
-		(1 << IIO_CHAN_INFO_SCALE_SHARED),
+		(1 << IIO_CHAN_INFO_SCALE_SHARED) |
+		(1 << IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY_SHARED),
 		.address = accel_x,
 		.scan_index = ADIS16400_SCAN_ACC_X,
 		.scan_type = IIO_ST('s', 14, 16, 0),
@@ -894,7 +995,8 @@ static const struct iio_chan_spec adis16334_channels[] = {
 		.modified = 1,
 		.channel2 = IIO_MOD_Y,
 		.info_mask = (1 << IIO_CHAN_INFO_CALIBBIAS_SEPARATE) |
-		(1 << IIO_CHAN_INFO_SCALE_SHARED),
+		(1 << IIO_CHAN_INFO_SCALE_SHARED) |
+		(1 << IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY_SHARED),
 		.address = accel_y,
 		.scan_index = ADIS16400_SCAN_ACC_Y,
 		.scan_type = IIO_ST('s', 14, 16, 0),
@@ -903,7 +1005,8 @@ static const struct iio_chan_spec adis16334_channels[] = {
 		.modified = 1,
 		.channel2 = IIO_MOD_Z,
 		.info_mask = (1 << IIO_CHAN_INFO_CALIBBIAS_SEPARATE) |
-		(1 << IIO_CHAN_INFO_SCALE_SHARED),
+		(1 << IIO_CHAN_INFO_SCALE_SHARED) |
+		(1 << IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY_SHARED),
 		.address = accel_z,
 		.scan_index = ADIS16400_SCAN_ACC_Z,
 		.scan_type = IIO_ST('s', 14, 16, 0),
-- 
1.7.3.4

Re: [RFC PATCH 0/2] IIO: Filtering - how to handle.

[JohnLM]

On Fri, 30 Sep 2011 11:17:57 +0100
Jonathan Cameron <jic23@cam.ac.uk> wrote:

> Hi All,
> 
> One big area we have pretty much glossed over so far is devices with
> controllable hardware filters.  This RFC proposes one option for
> how to handle this. For low pass filters at least, the 3db point
> seems the obvious choice as it allows us to gloss over exactly
> what type of filter it is whilst still capturing it's basic
> property of what it lets through.
> 
> What do people think?
I have no exact filters in mind but in general since they affect the
readings I think some kind of framework is needed.
Some generic types should be defined, but even when IIO can't make out
what kind of filter it is, user-space app might so it should be exposed.

For filters that can be enabled/disabled userspace could even
abstract the filtering routine to use hardware filter when desired or
use software filter when hardware filter is not present or is
unreliable.

The implementation is a whole other story I don't have time to think
about right now. :)

> 
> Jonathan
> 
> Jonathan Cameron (2):
>   staging:iio: filter description - low pass 3db frequency.
>   staging:iio:imu:adis16400 add control of data filtering.
> 
>  drivers/staging/iio/iio.h                |    2 +
>  drivers/staging/iio/imu/adis16400.h      |    2 +
>  drivers/staging/iio/imu/adis16400_core.c |  177
> +++++++++++++++++++++++------
> drivers/staging/iio/industrialio-core.c  |    2 + 4 files changed,
> 146 insertions(+), 37 deletions(-)
> 

Re: [RFC PATCH 0/2] IIO: Filtering - how to handle.

[Jonathan Cameron]

On 10/04/11 13:39, JohnLM wrote:
> On Fri, 30 Sep 2011 11:17:57 +0100
> Jonathan Cameron <jic23@cam.ac.uk> wrote:
> 
>> Hi All,
>>
>> One big area we have pretty much glossed over so far is devices with
>> controllable hardware filters.  This RFC proposes one option for
>> how to handle this. For low pass filters at least, the 3db point
>> seems the obvious choice as it allows us to gloss over exactly
>> what type of filter it is whilst still capturing it's basic
>> property of what it lets through.
>>
>> What do people think?
> I have no exact filters in mind but in general since they affect the
> readings I think some kind of framework is needed.
> Some generic types should be defined, but even when IIO can't make out
> what kind of filter it is, user-space app might so it should be exposed.
True.  But how generic do we want and how do we specify it?

The version here assumes that knowing it is low pass is enough, but
is that true.  Do we need to know for example if it is a Butterworth filter?

If we do, does it need to be in the naming?   If not, should we perhaps
have in_voltage_filter_low_pass_type with named filter types?  Perhaps
this also has say butterworth-N for N tap butterworth for example?
Or should be have filter_low_pass_type and filter_low_pass_taps?
Number of taps is often tied up with the 3db point, which would make
things a little interesting.  We might have to buffer the 3db request
and do a hardware update on any of type, taps, 3db_frequency and
sampling frequency changing.  We are already doing that on frequency
in the example given here, so not so bad.

> 
> For filters that can be enabled/disabled userspace could even
> abstract the filtering routine to use hardware filter when desired or
> use software filter when hardware filter is not present or is
> unreliable.
I'm unconvinced that we want to do software filtering in kernel land.
Can't immediately see the point, unless possibly it was used to do
data reduction into a buffer.  Worth keeping in mind, but no something
I can see happening any time soon.
> 
> The implementation is a whole other story I don't have time to think
> about right now. :)
:)
> 
>>
>> Jonathan
>>
>> Jonathan Cameron (2):
>>   staging:iio: filter description - low pass 3db frequency.
>>   staging:iio:imu:adis16400 add control of data filtering.
>>
>>  drivers/staging/iio/iio.h                |    2 +
>>  drivers/staging/iio/imu/adis16400.h      |    2 +
>>  drivers/staging/iio/imu/adis16400_core.c |  177
>> +++++++++++++++++++++++------
>> drivers/staging/iio/industrialio-core.c  |    2 + 4 files changed,
>> 146 insertions(+), 37 deletions(-)
>>
> 
> 

Re: [RFC PATCH 0/2] IIO: Filtering - how to handle.

[JohnLM]

On Tue, 04 Oct 2011 15:23:42 +0100
Jonathan Cameron <jic23@cam.ac.uk> wrote:

> On 10/04/11 13:39, JohnLM wrote:
> > On Fri, 30 Sep 2011 11:17:57 +0100
> > Jonathan Cameron <jic23@cam.ac.uk> wrote:
> > 
> >> Hi All,
> >>
> >> One big area we have pretty much glossed over so far is devices
> >> with controllable hardware filters.  This RFC proposes one option
> >> for how to handle this. For low pass filters at least, the 3db
> >> point seems the obvious choice as it allows us to gloss over
> >> exactly what type of filter it is whilst still capturing it's basic
> >> property of what it lets through.
> >>
> >> What do people think?
> > I have no exact filters in mind but in general since they affect the
> > readings I think some kind of framework is needed.
> > Some generic types should be defined, but even when IIO can't make
> > out what kind of filter it is, user-space app might so it should be
> > exposed.
> True.  But how generic do we want and how do we specify it?
> 
> The version here assumes that knowing it is low pass is enough, but
> is that true.  Do we need to know for example if it is a Butterworth
> filter?
IMO it makes most sense we categorize them by
basic functionality. Thus making it 'low pass filter' should be good
enough, but we can (and should) expose it's type.
> 
> If we do, does it need to be in the naming?   If not, should we
> perhaps have in_voltage_filter_low_pass_type with named filter
> types?  Perhaps this also has say butterworth-N for N tap butterworth
> for example? Or should be have filter_low_pass_type and
> filter_low_pass_taps? Number of taps is often tied up with the 3db
> point, which would make things a little interesting.  We might have
> to buffer the 3db request and do a hardware update on any of type,
> taps, 3db_frequency and sampling frequency changing.  We are already
> doing that on frequency in the example given here, so not so bad.
> 
> > 
> > For filters that can be enabled/disabled userspace could even
> > abstract the filtering routine to use hardware filter when desired
> > or use software filter when hardware filter is not present or is
> > unreliable.
> I'm unconvinced that we want to do software filtering in kernel land.
> Can't immediately see the point, unless possibly it was used to do
> data reduction into a buffer.  Worth keeping in mind, but no something
> I can see happening any time soon.
I ran a bit ahead of me. I didn't intend to do this kind of processing
in kernel space. Surely software filters should stay in userspace, I
don't see the point of this being in kernel space either.
What I meant was that enough information about filters must be exposed
so this abstraction could be done in userspace library or/and app.

> > 
> > The implementation is a whole other story I don't have time to think
> > about right now. :)
> :)
> > 
> >>
> >> Jonathan
> >>
> >> Jonathan Cameron (2):
> >>   staging:iio: filter description - low pass 3db frequency.
> >>   staging:iio:imu:adis16400 add control of data filtering.
> >>
> >>  drivers/staging/iio/iio.h                |    2 +
> >>  drivers/staging/iio/imu/adis16400.h      |    2 +
> >>  drivers/staging/iio/imu/adis16400_core.c |  177
> >> +++++++++++++++++++++++------
> >> drivers/staging/iio/industrialio-core.c  |    2 + 4 files changed,
> >> 146 insertions(+), 37 deletions(-)
> >>
> > 
> > 
> 

Re: [RFC PATCH 0/2] IIO: Filtering - how to handle.

[Jonathan Cameron]

On 10/06/11 15:01, JohnLM wrote:
> On Tue, 04 Oct 2011 15:23:42 +0100
> Jonathan Cameron <jic23@cam.ac.uk> wrote:
> 
>> On 10/04/11 13:39, JohnLM wrote:
>>> On Fri, 30 Sep 2011 11:17:57 +0100
>>> Jonathan Cameron <jic23@cam.ac.uk> wrote:
>>>
>>>> Hi All,
>>>>
>>>> One big area we have pretty much glossed over so far is devices
>>>> with controllable hardware filters.  This RFC proposes one option
>>>> for how to handle this. For low pass filters at least, the 3db
>>>> point seems the obvious choice as it allows us to gloss over
>>>> exactly what type of filter it is whilst still capturing it's basic
>>>> property of what it lets through.
>>>>
>>>> What do people think?
>>> I have no exact filters in mind but in general since they affect the
>>> readings I think some kind of framework is needed.
>>> Some generic types should be defined, but even when IIO can't make
>>> out what kind of filter it is, user-space app might so it should be
>>> exposed.
>> True.  But how generic do we want and how do we specify it?
>>
>> The version here assumes that knowing it is low pass is enough, but
>> is that true.  Do we need to know for example if it is a Butterworth
>> filter?
> IMO it makes most sense we categorize them by
> basic functionality. Thus making it 'low pass filter' should be good
> enough, but we can (and should) expose it's type.
Hmm. Going to be interesting to document these.  Your Butterworth
etc classes are well known, but there are some nasty hacky things
out there as well.  Lets keep it as a single sysfs type file when we
do it (not suggesting doing this now unless we have a user).
So the butterworth-2, butterworth-4, butterworth-8 etc here.
They are least almost a single conceptual value...
>>
>> If we do, does it need to be in the naming?   If not, should we
>> perhaps have in_voltage_filter_low_pass_type with named filter
>> types?  Perhaps this also has say butterworth-N for N tap butterworth
>> for example? Or should be have filter_low_pass_type and
>> filter_low_pass_taps? Number of taps is often tied up with the 3db
>> point, which would make things a little interesting.  We might have
>> to buffer the 3db request and do a hardware update on any of type,
>> taps, 3db_frequency and sampling frequency changing.  We are already
>> doing that on frequency in the example given here, so not so bad.
>>
>>>
>>> For filters that can be enabled/disabled userspace could even
>>> abstract the filtering routine to use hardware filter when desired
>>> or use software filter when hardware filter is not present or is
>>> unreliable.
>> I'm unconvinced that we want to do software filtering in kernel land.
>> Can't immediately see the point, unless possibly it was used to do
>> data reduction into a buffer.  Worth keeping in mind, but no something
>> I can see happening any time soon.
> I ran a bit ahead of me. I didn't intend to do this kind of processing
> in kernel space. Surely software filters should stay in userspace, I
> don't see the point of this being in kernel space either.
> What I meant was that enough information about filters must be exposed
> so this abstraction could be done in userspace library or/and app.
Agreed.  Though I an inclined to say - add it when needed rather
than anticipate the need. For now the 3db point works for anything I know
of.

Jonathan

RE: [RFC PATCH 0/2] IIO: Filtering - how to handle.

[Hennerich, Michael]

Jonathan Cameron wrote on 2011-09-30:
> Hi All,
>
> One big area we have pretty much glossed over so far is devices with
> controllable hardware filters.  This RFC proposes one option for
> how to handle this. For low pass filters at least, the 3db point
> seems the obvious choice as it allows us to gloss over exactly
> what type of filter it is whilst still capturing it's basic
> property of what it lets through.
>
> What do people think?

Looks good. The 3dB cutoff or corner frequency is typically used to describ=
e
various types of filters: low, high or band (pass/stop).

Documentation?


>
> Jonathan
>
> Jonathan Cameron (2):
>   staging:iio: filter description - low pass 3db frequency.
>   staging:iio:imu:adis16400 add control of data filtering.
>  drivers/staging/iio/iio.h                |    2 +
>  drivers/staging/iio/imu/adis16400.h      |    2 +
>  drivers/staging/iio/imu/adis16400_core.c |  177
>  +++++++++++++++++++++++------ drivers/staging/iio/industrialio-core.c
>  |    2 + 4 files changed, 146 insertions(+), 37 deletions(-)

Greetings,
Michael

--
Analog Devices GmbH      Wilhelm-Wagenfeld-Str. 6      80807 Muenchen
Sitz der Gesellschaft: Muenchen; Registergericht: Muenchen HRB 40368;
Geschaeftsfuehrer:Dr.Carsten Suckrow, Thomas Wessel, William A. Martin, Mar=
garet Seif

Re: [RFC PATCH 0/2] IIO: Filtering - how to handle.

[Jonathan Cameron]

On 10/05/11 07:55, Hennerich, Michael wrote:
> Jonathan Cameron wrote on 2011-09-30:
>> Hi All,
>>
>> One big area we have pretty much glossed over so far is devices with
>> controllable hardware filters.  This RFC proposes one option for
>> how to handle this. For low pass filters at least, the 3db point
>> seems the obvious choice as it allows us to gloss over exactly
>> what type of filter it is whilst still capturing it's basic
>> property of what it lets through.
>>
>> What do people think?
> 
> Looks good. The 3dB cutoff or corner frequency is typically used to describe
> various types of filters: low, high or band (pass/stop).
> 
> Documentation?
Indeed. Naughty me ;)  Will add for new version.
> 
> 
>>
>> Jonathan
>>
>> Jonathan Cameron (2):
>>   staging:iio: filter description - low pass 3db frequency.
>>   staging:iio:imu:adis16400 add control of data filtering.
>>  drivers/staging/iio/iio.h                |    2 +
>>  drivers/staging/iio/imu/adis16400.h      |    2 +
>>  drivers/staging/iio/imu/adis16400_core.c |  177
>>  +++++++++++++++++++++++------ drivers/staging/iio/industrialio-core.c
>>  |    2 + 4 files changed, 146 insertions(+), 37 deletions(-)
> 
> Greetings,
> Michael
> 
> --
> Analog Devices GmbH      Wilhelm-Wagenfeld-Str. 6      80807 Muenchen
> Sitz der Gesellschaft: Muenchen; Registergericht: Muenchen HRB 40368;
> Geschaeftsfuehrer:Dr.Carsten Suckrow, Thomas Wessel, William A. Martin, Margaret Seif
> 
> 
> 
> 

[PATCH] staging:iio:documentation: document filter_low_pass_3db_frequency.

[Jonathan Cameron]

Add the sysfs ABI docs for all current instances of this
parameter.

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

diff --git a/drivers/staging/iio/Documentation/sysfs-bus-iio b/drivers/staging/iio/Documentation/sysfs-bus-iio
index 0d6823d..46a995d 100644
--- a/drivers/staging/iio/Documentation/sysfs-bus-iio
+++ b/drivers/staging/iio/Documentation/sysfs-bus-iio
@@ -276,6 +276,16 @@ Description:
 		If a discrete set of scale values are available, they
 		are listed in this attribute.
 
+What:		/sys/.../in_accel_filter_low_pass_3db_frequency
+What:		/sys/.../in_magn_filter_low_pass_3db_frequency
+What:		/sys/.../in_anglvel_filter_low_pass_3db_frequency
+KernelVersion:	3.2
+Contact:	linux-iio@vger.kernel.org
+Description:
+		If a known or controllable low pass filter is applied
+		to the underlying data channel, then this parameter
+		gives the 3dB frequency of the filter in Hz.
+
 What:		/sys/bus/iio/devices/iio:deviceX/out_voltageY_raw
 KernelVersion:	2.6.37
 Contact:	linux-iio@vger.kernel.org
-- 
1.7.3.4