[PATCH 0/3] adis16xxx: fix some minor issues and clean-up

[PATCH 0/3] adis16xxx: fix some minor issues and clean-up

[Barry Song]

Hi Jonathan,
As i will fetch some iio ring functions to iio common layer, i'd like that
some old patches, which maybe are related with iio ring,  are sent at first
to avoid possible mis-order.

1. adis16300: fix some minor issues and clean-up
2. adis16400: fix some minor issues and clean-up
3. adis16209/220/240/350: tuning spi delay to make hardware more stable based on test

Thanks
Barry


 		     

[PATCH 1/3] adis16300: fix some minor issues and clean-up

[Barry Song]

1. add delay between spi transfers
2. move burst read to ring function
3. clean-up

Signed-off-by: Barry Song <21cnbao@gmail.com>
---
 drivers/staging/iio/imu/adis16300.h      |    6 -
 drivers/staging/iio/imu/adis16300_core.c |  151 +++++++++++++-----------------
 drivers/staging/iio/imu/adis16300_ring.c |   54 ++++++++++-
 3 files changed, 119 insertions(+), 95 deletions(-)

diff --git a/drivers/staging/iio/imu/adis16300.h b/drivers/staging/iio/imu/adis16300.h
index 1c7ea5c..b050067 100644
--- a/drivers/staging/iio/imu/adis16300.h
+++ b/drivers/staging/iio/imu/adis16300.h
@@ -115,14 +115,8 @@ struct adis16300_state {
 	struct mutex			buf_lock;
 };
 
-int adis16300_spi_read_burst(struct device *dev, u8 *rx);
-
 int adis16300_set_irq(struct device *dev, bool enable);
 
-int adis16300_reset(struct device *dev);
-
-int adis16300_check_status(struct device *dev);
-
 #ifdef CONFIG_IIO_RING_BUFFER
 /* At the moment triggers are only used for ring buffer
  * filling. This may change!
diff --git a/drivers/staging/iio/imu/adis16300_core.c b/drivers/staging/iio/imu/adis16300_core.c
index 5a7e5ef..28667e8 100644
--- a/drivers/staging/iio/imu/adis16300_core.c
+++ b/drivers/staging/iio/imu/adis16300_core.c
@@ -29,10 +29,7 @@
 
 #define DRIVER_NAME		"adis16300"
 
-/* At the moment the spi framework doesn't allow global setting of cs_change.
- * It's in the likely to be added comment at the top of spi.h.
- * This means that use cannot be made of spi_write etc.
- */
+static int adis16300_check_status(struct device *dev);
 
 /**
  * adis16300_spi_write_reg_8() - write single byte to a register
@@ -79,11 +76,13 @@ static int adis16300_spi_write_reg_16(struct device *dev,
 			.bits_per_word = 8,
 			.len = 2,
 			.cs_change = 1,
+			.delay_usecs = 75,
 		}, {
 			.tx_buf = st->tx + 2,
 			.bits_per_word = 8,
 			.len = 2,
 			.cs_change = 1,
+			.delay_usecs = 75,
 		},
 	};
 
@@ -122,12 +121,14 @@ static int adis16300_spi_read_reg_16(struct device *dev,
 			.tx_buf = st->tx,
 			.bits_per_word = 8,
 			.len = 2,
-			.cs_change = 0,
+			.cs_change = 1,
+			.delay_usecs = 75,
 		}, {
 			.rx_buf = st->rx,
 			.bits_per_word = 8,
 			.len = 2,
-			.cs_change = 0,
+			.cs_change = 1,
+			.delay_usecs = 75,
 		},
 	};
 
@@ -154,54 +155,6 @@ error_ret:
 	return ret;
 }
 
-/**
- * adis16300_spi_read_burst() - read all data registers
- * @dev: device associated with child of actual device (iio_dev or iio_trig)
- * @rx: somewhere to pass back the value read (min size is 24 bytes)
- **/
-int adis16300_spi_read_burst(struct device *dev, u8 *rx)
-{
-	struct spi_message msg;
-	struct iio_dev *indio_dev = dev_get_drvdata(dev);
-	struct adis16300_state *st = iio_dev_get_devdata(indio_dev);
-	u32 old_speed_hz = st->us->max_speed_hz;
-	int ret;
-
-	struct spi_transfer xfers[] = {
-		{
-			.tx_buf = st->tx,
-			.bits_per_word = 8,
-			.len = 2,
-			.cs_change = 0,
-		}, {
-			.rx_buf = rx,
-			.bits_per_word = 8,
-			.len = 18,
-			.cs_change = 0,
-		},
-	};
-
-	mutex_lock(&st->buf_lock);
-	st->tx[0] = ADIS16300_READ_REG(ADIS16300_GLOB_CMD);
-	st->tx[1] = 0;
-
-	spi_message_init(&msg);
-	spi_message_add_tail(&xfers[0], &msg);
-	spi_message_add_tail(&xfers[1], &msg);
-
-	st->us->max_speed_hz = min(ADIS16300_SPI_BURST, old_speed_hz);
-	spi_setup(st->us);
-
-	ret = spi_sync(st->us, &msg);
-	if (ret)
-		dev_err(&st->us->dev, "problem when burst reading");
-
-	st->us->max_speed_hz = old_speed_hz;
-	spi_setup(st->us);
-	mutex_unlock(&st->buf_lock);
-	return ret;
-}
-
 static ssize_t adis16300_spi_read_signed(struct device *dev,
 		struct device_attribute *attr,
 		char *buf,
@@ -240,6 +193,24 @@ static ssize_t adis16300_read_12bit_unsigned(struct device *dev,
 	return sprintf(buf, "%u\n", val & 0x0FFF);
 }
 
+static ssize_t adis16300_read_14bit_unsigned(struct device *dev,
+		struct device_attribute *attr,
+		char *buf)
+{
+	int ret;
+	u16 val = 0;
+	struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
+
+	ret = adis16300_spi_read_reg_16(dev, this_attr->address, &val);
+	if (ret)
+		return ret;
+
+	if (val & ADIS16300_ERROR_ACTIVE)
+		adis16300_check_status(dev);
+
+	return sprintf(buf, "%u\n", val & 0x3FFF);
+}
+
 static ssize_t adis16300_read_14bit_signed(struct device *dev,
 		struct device_attribute *attr,
 		char *buf)
@@ -356,6 +327,18 @@ static ssize_t adis16300_write_frequency(struct device *dev,
 	return ret ? ret : len;
 }
 
+static int adis16300_reset(struct device *dev)
+{
+	int ret;
+	ret = adis16300_spi_write_reg_8(dev,
+			ADIS16300_GLOB_CMD,
+			ADIS16300_GLOB_CMD_SW_RESET);
+	if (ret)
+		dev_err(dev, "problem resetting device");
+
+	return ret;
+}
+
 static ssize_t adis16300_write_reset(struct device *dev,
 		struct device_attribute *attr,
 		const char *buf, size_t len)
@@ -371,8 +354,6 @@ static ssize_t adis16300_write_reset(struct device *dev,
 	return -1;
 }
 
-
-
 int adis16300_set_irq(struct device *dev, bool enable)
 {
 	int ret;
@@ -396,32 +377,37 @@ error_ret:
 	return ret;
 }
 
-int adis16300_reset(struct device *dev)
+/* Power down the device */
+static int adis16300_stop_device(struct device *dev)
 {
 	int ret;
-	ret = adis16300_spi_write_reg_8(dev,
-			ADIS16300_GLOB_CMD,
-			ADIS16300_GLOB_CMD_SW_RESET);
+	u16 val = ADIS16300_SLP_CNT_POWER_OFF;
+
+	ret = adis16300_spi_write_reg_16(dev, ADIS16300_SLP_CNT, val);
 	if (ret)
-		dev_err(dev, "problem resetting device");
+		dev_err(dev, "problem with turning device off: SLP_CNT");
 
 	return ret;
 }
 
-/* Power down the device */
-static int adis16300_stop_device(struct device *dev)
+static int adis16300_self_test(struct device *dev)
 {
 	int ret;
-	u16 val = ADIS16300_SLP_CNT_POWER_OFF;
+	ret = adis16300_spi_write_reg_16(dev,
+			ADIS16300_MSC_CTRL,
+			ADIS16300_MSC_CTRL_MEM_TEST);
+	if (ret) {
+		dev_err(dev, "problem starting self test");
+		goto err_ret;
+	}
 
-	ret = adis16300_spi_write_reg_16(dev, ADIS16300_SLP_CNT, val);
-	if (ret)
-		dev_err(dev, "problem with turning device off: SLP_CNT");
+	adis16300_check_status(dev);
 
+err_ret:
 	return ret;
 }
 
-int adis16300_check_status(struct device *dev)
+static int adis16300_check_status(struct device *dev)
 {
 	u16 status;
 	int ret;
@@ -483,6 +469,11 @@ static int adis16300_initial_setup(struct adis16300_state *st)
 	}
 
 	/* Do self test */
+	ret = adis16300_self_test(dev);
+	if (ret) {
+		dev_err(dev, "self test failure");
+		goto err_ret;
+	}
 
 	/* Read status register to check the result */
 	ret = adis16300_check_status(dev);
@@ -526,7 +517,7 @@ static IIO_DEV_ATTR_ACCEL_Z_OFFSET(S_IWUSR | S_IRUGO,
 		adis16300_write_16bit,
 		ADIS16300_ZACCL_OFF);
 
-static IIO_DEV_ATTR_IN_NAMED_RAW(supply, adis16300_read_14bit_signed,
+static IIO_DEV_ATTR_IN_NAMED_RAW(supply, adis16300_read_14bit_unsigned,
 			   ADIS16300_SUPPLY_OUT);
 static IIO_CONST_ATTR(in_supply_scale, "0.00242");
 
@@ -548,7 +539,7 @@ static IIO_DEV_ATTR_INCLI_Y(adis16300_read_13bit_signed,
 		ADIS16300_YINCLI_OUT);
 static IIO_CONST_ATTR(incli_scale, "0.044 d");
 
-static IIO_DEV_ATTR_TEMP_RAW(adis16300_read_12bit_signed);
+static IIO_DEV_ATTR_TEMP_RAW(adis16300_read_12bit_unsigned);
 static IIO_CONST_ATTR(temp_offset, "198.16 K");
 static IIO_CONST_ATTR(temp_scale, "0.14 K");
 
@@ -659,15 +650,7 @@ static int __devinit adis16300_probe(struct spi_device *spi)
 		goto error_unreg_ring_funcs;
 	}
 
-	if (spi->irq && gpio_is_valid(irq_to_gpio(spi->irq)) > 0) {
-#if 0 /* fixme: here we should support */
-		iio_init_work_cont(&st->work_cont_thresh,
-				NULL,
-				adis16300_thresh_handler_bh_no_check,
-				0,
-				0,
-				st);
-#endif
+	if (spi->irq) {
 		ret = iio_register_interrupt_line(spi->irq,
 				st->indio_dev,
 				0,
@@ -688,10 +671,9 @@ static int __devinit adis16300_probe(struct spi_device *spi)
 	return 0;
 
 error_remove_trigger:
-	if (st->indio_dev->modes & INDIO_RING_TRIGGERED)
-		adis16300_remove_trigger(st->indio_dev);
+	adis16300_remove_trigger(st->indio_dev);
 error_unregister_line:
-	if (st->indio_dev->modes & INDIO_RING_TRIGGERED)
+	if (spi->irq)
 		iio_unregister_interrupt_line(st->indio_dev, 0);
 error_uninitialize_ring:
 	adis16300_uninitialize_ring(st->indio_dev->ring);
@@ -712,7 +694,6 @@ error_ret:
 	return ret;
 }
 
-/* fixme, confirm ordering in this function */
 static int adis16300_remove(struct spi_device *spi)
 {
 	int ret;
@@ -726,12 +707,12 @@ static int adis16300_remove(struct spi_device *spi)
 	flush_scheduled_work();
 
 	adis16300_remove_trigger(indio_dev);
-	if (spi->irq && gpio_is_valid(irq_to_gpio(spi->irq)) > 0)
+	if (spi->irq)
 		iio_unregister_interrupt_line(indio_dev, 0);
 
 	adis16300_uninitialize_ring(indio_dev->ring);
-	adis16300_unconfigure_ring(indio_dev);
 	iio_device_unregister(indio_dev);
+	adis16300_unconfigure_ring(indio_dev);
 	kfree(st->tx);
 	kfree(st->rx);
 	kfree(st);
diff --git a/drivers/staging/iio/imu/adis16300_ring.c b/drivers/staging/iio/imu/adis16300_ring.c
index 76cf8a6..17ceb72 100644
--- a/drivers/staging/iio/imu/adis16300_ring.c
+++ b/drivers/staging/iio/imu/adis16300_ring.c
@@ -26,7 +27,7 @@ static inline u16 combine_8_to_16(u8 lower, u8 upper)
 	return _lower | (_upper << 8);
 }
 
-static IIO_SCAN_EL_C(supply, ADIS16300_SCAN_SUPPLY, IIO_SIGNED(14),
+static IIO_SCAN_EL_C(supply, ADIS16300_SCAN_SUPPLY, IIO_UNSIGNED(14),
 		     ADIS16300_SUPPLY_OUT, NULL);
 
 static IIO_SCAN_EL_C(gyro_x, ADIS16300_SCAN_GYRO_X, IIO_SIGNED(14),
@@ -39,9 +40,9 @@ static IIO_SCAN_EL_C(accel_y, ADIS16300_SCAN_ACC_Y, IIO_SIGNED(14),
 static IIO_SCAN_EL_C(accel_z, ADIS16300_SCAN_ACC_Z, IIO_SIGNED(14),
 		     ADIS16300_ZACCL_OUT, NULL);
 
-static IIO_SCAN_EL_C(temp, ADIS16300_SCAN_TEMP, IIO_SIGNED(12),
+static IIO_SCAN_EL_C(temp, ADIS16300_SCAN_TEMP, IIO_UNSIGNED(12),
 		     ADIS16300_TEMP_OUT, NULL);
-static IIO_SCAN_EL_C(adc_0, ADIS16300_SCAN_ADC_0, IIO_SIGNED(12),
+static IIO_SCAN_EL_C(adc_0, ADIS16300_SCAN_ADC_0, IIO_UNSIGNED(12),
 		     ADIS16300_AUX_ADC, NULL);
 
 static IIO_SCAN_EL_C(incli_x, ADIS16300_SCAN_INCLI_X, IIO_SIGNED(12),
@@ -87,6 +88,54 @@ static void adis16300_poll_func_th(struct iio_dev *indio_dev)
 	 */
 }
 
+/**
+ * adis16300_spi_read_burst() - read all data registers
+ * @dev: device associated with child of actual device (iio_dev or iio_trig)
+ * @rx: somewhere to pass back the value read (min size is 24 bytes)
+ **/
+static int adis16300_spi_read_burst(struct device *dev, u8 *rx)
+{
+	struct spi_message msg;
+	struct iio_dev *indio_dev = dev_get_drvdata(dev);
+	struct adis16300_state *st = iio_dev_get_devdata(indio_dev);
+	u32 old_speed_hz = st->us->max_speed_hz;
+	int ret;
+
+	struct spi_transfer xfers[] = {
+		{
+			.tx_buf = st->tx,
+			.bits_per_word = 8,
+			.len = 2,
+			.cs_change = 0,
+		}, {
+			.rx_buf = rx,
+			.bits_per_word = 8,
+			.len = 18,
+			.cs_change = 0,
+		},
+	};
+
+	mutex_lock(&st->buf_lock);
+	st->tx[0] = ADIS16300_READ_REG(ADIS16300_GLOB_CMD);
+	st->tx[1] = 0;
+
+	spi_message_init(&msg);
+	spi_message_add_tail(&xfers[0], &msg);
+	spi_message_add_tail(&xfers[1], &msg);
+
+	st->us->max_speed_hz = ADIS16300_SPI_BURST;
+	spi_setup(st->us);
+
+	ret = spi_sync(st->us, &msg);
+	if (ret)
+		dev_err(&st->us->dev, "problem when burst reading");
+
+	st->us->max_speed_hz = old_speed_hz;
+	spi_setup(st->us);
+	mutex_unlock(&st->buf_lock);
+	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.
  */
-- 
1.5.6.3

[PATCH 2/3] adis16400: fix some minor issues and clean-up

[Barry Song]

1. move adis16400_spi_read_burst() to adis16400_ring.c since it is only
   called there
2. add the lost calling to adis16400_self_test()
3. codes cleanup

Signed-off-by: Barry Song <21cnbao@gmail.com>
---
 drivers/staging/iio/imu/adis16400.h      |    6 --
 drivers/staging/iio/imu/adis16400_core.c |   84 +++++++----------------------
 drivers/staging/iio/imu/adis16400_ring.c |   48 +++++++++++++++++
 3 files changed, 70 insertions(+), 71 deletions(-)

diff --git a/drivers/staging/iio/imu/adis16400.h b/drivers/staging/iio/imu/adis16400.h
index 5a69a7a..04bae36 100644
--- a/drivers/staging/iio/imu/adis16400.h
+++ b/drivers/staging/iio/imu/adis16400.h
@@ -147,14 +147,8 @@ struct adis16400_state {
 	struct mutex			buf_lock;
 };
 
-int adis16400_spi_read_burst(struct device *dev, u8 *rx);
-
 int adis16400_set_irq(struct device *dev, bool enable);
 
-int adis16400_reset(struct device *dev);
-
-int adis16400_check_status(struct device *dev);
-
 #ifdef CONFIG_IIO_RING_BUFFER
 /* At the moment triggers are only used for ring buffer
  * filling. This may change!
diff --git a/drivers/staging/iio/imu/adis16400_core.c b/drivers/staging/iio/imu/adis16400_core.c
index e69e2ce..a668a90 100644
--- a/drivers/staging/iio/imu/adis16400_core.c
+++ b/drivers/staging/iio/imu/adis16400_core.c
@@ -36,6 +36,8 @@
 
 #define DRIVER_NAME		"adis16400"
 
+static int adis16400_check_status(struct device *dev);
+
 /* At the moment the spi framework doesn't allow global setting of cs_change.
  * It's in the likely to be added comment at the top of spi.h.
  * This means that use cannot be made of spi_write etc.
@@ -161,54 +163,6 @@ error_ret:
 	return ret;
 }
 
-/**
- * adis16400_spi_read_burst() - read all data registers
- * @dev: device associated with child of actual device (iio_dev or iio_trig)
- * @rx: somewhere to pass back the value read (min size is 24 bytes)
- **/
-int adis16400_spi_read_burst(struct device *dev, u8 *rx)
-{
-	struct spi_message msg;
-	struct iio_dev *indio_dev = dev_get_drvdata(dev);
-	struct adis16400_state *st = iio_dev_get_devdata(indio_dev);
-	u32 old_speed_hz = st->us->max_speed_hz;
-	int ret;
-
-	struct spi_transfer xfers[] = {
-		{
-			.tx_buf = st->tx,
-			.bits_per_word = 8,
-			.len = 2,
-			.cs_change = 0,
-		}, {
-			.rx_buf = rx,
-			.bits_per_word = 8,
-			.len = 24,
-			.cs_change = 1,
-		},
-	};
-
-	mutex_lock(&st->buf_lock);
-	st->tx[0] = ADIS16400_READ_REG(ADIS16400_GLOB_CMD);
-	st->tx[1] = 0;
-
-	spi_message_init(&msg);
-	spi_message_add_tail(&xfers[0], &msg);
-	spi_message_add_tail(&xfers[1], &msg);
-
-	st->us->max_speed_hz = min(ADIS16400_SPI_BURST, old_speed_hz);
-	spi_setup(st->us);
-
-	ret = spi_sync(st->us, &msg);
-	if (ret)
-		dev_err(&st->us->dev, "problem when burst reading");
-
-	st->us->max_speed_hz = old_speed_hz;
-	spi_setup(st->us);
-	mutex_unlock(&st->buf_lock);
-	return ret;
-}
-
 static ssize_t adis16400_spi_read_signed(struct device *dev,
 		struct device_attribute *attr,
 		char *buf,
@@ -277,7 +231,6 @@ static ssize_t adis16400_read_12bit_signed(struct device *dev,
 	return ret;
 }
 
-
 static ssize_t adis16400_write_16bit(struct device *dev,
 		struct device_attribute *attr,
 		const char *buf,
@@ -349,6 +302,18 @@ static ssize_t adis16400_write_frequency(struct device *dev,
 	return ret ? ret : len;
 }
 
+static int adis16400_reset(struct device *dev)
+{
+	int ret;
+	ret = adis16400_spi_write_reg_8(dev,
+			ADIS16400_GLOB_CMD,
+			ADIS16400_GLOB_CMD_SW_RESET);
+	if (ret)
+		dev_err(dev, "problem resetting device");
+
+	return ret;
+}
+
 static ssize_t adis16400_write_reset(struct device *dev,
 		struct device_attribute *attr,
 		const char *buf, size_t len)
@@ -364,8 +329,6 @@ static ssize_t adis16400_write_reset(struct device *dev,
 	return -1;
 }
 
-
-
 int adis16400_set_irq(struct device *dev, bool enable)
 {
 	int ret;
@@ -388,18 +351,6 @@ error_ret:
 	return ret;
 }
 
-int adis16400_reset(struct device *dev)
-{
-	int ret;
-	ret = adis16400_spi_write_reg_8(dev,
-			ADIS16400_GLOB_CMD,
-			ADIS16400_GLOB_CMD_SW_RESET);
-	if (ret)
-		dev_err(dev, "problem resetting device");
-
-	return ret;
-}
-
 /* Power down the device */
 static int adis16400_stop_device(struct device *dev)
 {
@@ -430,7 +381,7 @@ err_ret:
 	return ret;
 }
 
-int adis16400_check_status(struct device *dev)
+static int adis16400_check_status(struct device *dev)
 {
 	u16 status;
 	int ret;
@@ -496,6 +447,11 @@ static int adis16400_initial_setup(struct adis16400_state *st)
 	}
 
 	/* Do self test */
+	ret = adis16400_self_test(dev);
+	if (ret) {
+		dev_err(dev, "self test failure");
+		goto err_ret;
+	}
 
 	/* Read status register to check the result */
 	ret = adis16400_check_status(dev);
diff --git a/drivers/staging/iio/imu/adis16400_ring.c b/drivers/staging/iio/imu/adis16400_ring.c
index 5529b32..5d94cdc 100644
--- a/drivers/staging/iio/imu/adis16400_ring.c
+++ b/drivers/staging/iio/imu/adis16400_ring.c
@@ -96,6 +97,54 @@ static void adis16400_poll_func_th(struct iio_dev *indio_dev)
 	 */
 }
 
+/**
+ * adis16400_spi_read_burst() - read all data registers
+ * @dev: device associated with child of actual device (iio_dev or iio_trig)
+ * @rx: somewhere to pass back the value read (min size is 24 bytes)
+ **/
+static int adis16400_spi_read_burst(struct device *dev, u8 *rx)
+{
+	struct spi_message msg;
+	struct iio_dev *indio_dev = dev_get_drvdata(dev);
+	struct adis16400_state *st = iio_dev_get_devdata(indio_dev);
+	u32 old_speed_hz = st->us->max_speed_hz;
+	int ret;
+
+	struct spi_transfer xfers[] = {
+		{
+			.tx_buf = st->tx,
+			.bits_per_word = 8,
+			.len = 2,
+			.cs_change = 0,
+		}, {
+			.rx_buf = rx,
+			.bits_per_word = 8,
+			.len = 24,
+			.cs_change = 1,
+		},
+	};
+
+	mutex_lock(&st->buf_lock);
+	st->tx[0] = ADIS16400_READ_REG(ADIS16400_GLOB_CMD);
+	st->tx[1] = 0;
+
+	spi_message_init(&msg);
+	spi_message_add_tail(&xfers[0], &msg);
+	spi_message_add_tail(&xfers[1], &msg);
+
+	st->us->max_speed_hz = min(ADIS16400_SPI_BURST, old_speed_hz);
+	spi_setup(st->us);
+
+	ret = spi_sync(st->us, &msg);
+	if (ret)
+		dev_err(&st->us->dev, "problem when burst reading");
+
+	st->us->max_speed_hz = old_speed_hz;
+	spi_setup(st->us);
+	mutex_unlock(&st->buf_lock);
+	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.
  */
-- 
1.5.6.3

[PATCH 3/3] adis16209/220/240/350: tuning spi delay to make hardware more stable

[Barry Song]

Signed-off-by: Barry Song <21cnbao@gmail.com>
---
 drivers/staging/iio/accel/adis16209_core.c |    6 ++++--
 drivers/staging/iio/accel/adis16220_core.c |   12 ++++++------
 drivers/staging/iio/accel/adis16240_core.c |    8 ++++----
 drivers/staging/iio/imu/adis16350_core.c   |    8 ++++----
 4 files changed, 19 insertions(+), 17 deletions(-)

diff --git a/drivers/staging/iio/accel/adis16209_core.c b/drivers/staging/iio/accel/adis16209_core.c
index ac375c5..94491a1 100644
--- a/drivers/staging/iio/accel/adis16209_core.c
+++ b/drivers/staging/iio/accel/adis16209_core.c
@@ -76,11 +76,13 @@ static int adis16209_spi_write_reg_16(struct device *dev,
 			.bits_per_word = 8,
 			.len = 2,
 			.cs_change = 1,
+			.delay_usecs = 30,
 		}, {
 			.tx_buf = st->tx + 2,
 			.bits_per_word = 8,
 			.len = 2,
 			.cs_change = 1,
+			.delay_usecs = 30,
 		},
 	};
 
@@ -120,13 +122,13 @@ static int adis16209_spi_read_reg_16(struct device *dev,
 			.bits_per_word = 8,
 			.len = 2,
 			.cs_change = 1,
-			.delay_usecs = 20,
+			.delay_usecs = 30,
 		}, {
 			.rx_buf = st->rx,
 			.bits_per_word = 8,
 			.len = 2,
 			.cs_change = 1,
-			.delay_usecs = 20,
+			.delay_usecs = 30,
 		},
 	};
 
diff --git a/drivers/staging/iio/accel/adis16220_core.c b/drivers/staging/iio/accel/adis16220_core.c
index 6de439f..fffa49c 100644
--- a/drivers/staging/iio/accel/adis16220_core.c
+++ b/drivers/staging/iio/accel/adis16220_core.c
@@ -72,13 +72,13 @@ static int adis16220_spi_write_reg_16(struct device *dev,
 			.bits_per_word = 8,
 			.len = 2,
 			.cs_change = 1,
-			.delay_usecs = 25,
+			.delay_usecs = 35,
 		}, {
 			.tx_buf = st->tx + 2,
 			.bits_per_word = 8,
 			.len = 2,
 			.cs_change = 1,
-			.delay_usecs = 25,
+			.delay_usecs = 35,
 		},
 	};
 
@@ -118,13 +118,13 @@ static int adis16220_spi_read_reg_16(struct device *dev,
 			.bits_per_word = 8,
 			.len = 2,
 			.cs_change = 1,
-			.delay_usecs = 25,
+			.delay_usecs = 35,
 		}, {
 			.rx_buf = st->rx,
 			.bits_per_word = 8,
 			.len = 2,
 			.cs_change = 1,
-			.delay_usecs = 25,
+			.delay_usecs = 35,
 		},
 	};
 
@@ -291,9 +291,9 @@ static int adis16220_check_status(struct device *dev)
 	if (status & ADIS16220_DIAG_STAT_FLASH_UPT)
 		dev_err(dev, "Flash update failed\n");
 	if (status & ADIS16220_DIAG_STAT_POWER_HIGH)
-		dev_err(dev, "Power supply above 5.25V\n");
+		dev_err(dev, "Power supply above 3.625V\n");
 	if (status & ADIS16220_DIAG_STAT_POWER_LOW)
-		dev_err(dev, "Power supply below 4.75V\n");
+		dev_err(dev, "Power supply below 3.15V\n");
 
 error_ret:
 	return ret;
diff --git a/drivers/staging/iio/accel/adis16240_core.c b/drivers/staging/iio/accel/adis16240_core.c
index 54fd6d7..731bca6 100644
--- a/drivers/staging/iio/accel/adis16240_core.c
+++ b/drivers/staging/iio/accel/adis16240_core.c
@@ -74,13 +74,13 @@ static int adis16240_spi_write_reg_16(struct device *dev,
 			.bits_per_word = 8,
 			.len = 2,
 			.cs_change = 1,
-			.delay_usecs = 25,
+			.delay_usecs = 35,
 		}, {
 			.tx_buf = st->tx + 2,
 			.bits_per_word = 8,
 			.len = 2,
 			.cs_change = 1,
-			.delay_usecs = 25,
+			.delay_usecs = 35,
 		},
 	};
 
@@ -120,13 +120,13 @@ static int adis16240_spi_read_reg_16(struct device *dev,
 			.bits_per_word = 8,
 			.len = 2,
 			.cs_change = 1,
-			.delay_usecs = 25,
+			.delay_usecs = 35,
 		}, {
 			.rx_buf = st->rx,
 			.bits_per_word = 8,
 			.len = 2,
 			.cs_change = 1,
-			.delay_usecs = 25,
+			.delay_usecs = 35,
 		},
 	};
 
diff --git a/drivers/staging/iio/imu/adis16350_core.c b/drivers/staging/iio/imu/adis16350_core.c
index 0edde73..0bb19a9 100644
--- a/drivers/staging/iio/imu/adis16350_core.c
+++ b/drivers/staging/iio/imu/adis16350_core.c
@@ -75,13 +75,13 @@ static int adis16350_spi_write_reg_16(struct device *dev,
 			.bits_per_word = 8,
 			.len = 2,
 			.cs_change = 1,
-			.delay_usecs = 25,
+			.delay_usecs = 35,
 		}, {
 			.tx_buf = st->tx + 2,
 			.bits_per_word = 8,
 			.len = 2,
 			.cs_change = 1,
-			.delay_usecs = 25,
+			.delay_usecs = 35,
 		},
 	};
 
@@ -121,13 +121,13 @@ static int adis16350_spi_read_reg_16(struct device *dev,
 			.bits_per_word = 8,
 			.len = 2,
 			.cs_change = 1,
-			.delay_usecs = 25,
+			.delay_usecs = 35,
 		}, {
 			.rx_buf = st->rx,
 			.bits_per_word = 8,
 			.len = 2,
 			.cs_change = 1,
-			.delay_usecs = 25,
+			.delay_usecs = 35,
 		},
 	};
 
-- 
1.5.6.3

Re: [PATCH 3/3] adis16209/220/240/350: tuning spi delay to make hardware more stable

[Jonathan Cameron]

This clearly also fixes an incorrect message.   Obviously that is trivial
but it isn't apparent in commit message or comments.
> Signed-off-by: Barry Song <21cnbao@gmail.com>
Acked-by: Jonathan Cameron <jic23@cam.ac.uk>
> ---
>  drivers/staging/iio/accel/adis16209_core.c |    6 ++++--
>  drivers/staging/iio/accel/adis16220_core.c |   12 ++++++------
>  drivers/staging/iio/accel/adis16240_core.c |    8 ++++----
>  drivers/staging/iio/imu/adis16350_core.c   |    8 ++++----
>  4 files changed, 19 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/staging/iio/accel/adis16209_core.c b/drivers/staging/iio/accel/adis16209_core.c
> index ac375c5..94491a1 100644
> --- a/drivers/staging/iio/accel/adis16209_core.c
> +++ b/drivers/staging/iio/accel/adis16209_core.c
> @@ -76,11 +76,13 @@ static int adis16209_spi_write_reg_16(struct device *dev,
>  			.bits_per_word = 8,
>  			.len = 2,
>  			.cs_change = 1,
> +			.delay_usecs = 30,
>  		}, {
>  			.tx_buf = st->tx + 2,
>  			.bits_per_word = 8,
>  			.len = 2,
>  			.cs_change = 1,
> +			.delay_usecs = 30,
>  		},
>  	};
>  
> @@ -120,13 +122,13 @@ static int adis16209_spi_read_reg_16(struct device *dev,
>  			.bits_per_word = 8,
>  			.len = 2,
>  			.cs_change = 1,
> -			.delay_usecs = 20,
> +			.delay_usecs = 30,
>  		}, {
>  			.rx_buf = st->rx,
>  			.bits_per_word = 8,
>  			.len = 2,
>  			.cs_change = 1,
> -			.delay_usecs = 20,
> +			.delay_usecs = 30,
>  		},
>  	};
>  
> diff --git a/drivers/staging/iio/accel/adis16220_core.c b/drivers/staging/iio/accel/adis16220_core.c
> index 6de439f..fffa49c 100644
> --- a/drivers/staging/iio/accel/adis16220_core.c
> +++ b/drivers/staging/iio/accel/adis16220_core.c
> @@ -72,13 +72,13 @@ static int adis16220_spi_write_reg_16(struct device *dev,
>  			.bits_per_word = 8,
>  			.len = 2,
>  			.cs_change = 1,
> -			.delay_usecs = 25,
> +			.delay_usecs = 35,
>  		}, {
>  			.tx_buf = st->tx + 2,
>  			.bits_per_word = 8,
>  			.len = 2,
>  			.cs_change = 1,
> -			.delay_usecs = 25,
> +			.delay_usecs = 35,
>  		},
>  	};
>  
> @@ -118,13 +118,13 @@ static int adis16220_spi_read_reg_16(struct device *dev,
>  			.bits_per_word = 8,
>  			.len = 2,
>  			.cs_change = 1,
> -			.delay_usecs = 25,
> +			.delay_usecs = 35,
>  		}, {
>  			.rx_buf = st->rx,
>  			.bits_per_word = 8,
>  			.len = 2,
>  			.cs_change = 1,
> -			.delay_usecs = 25,
> +			.delay_usecs = 35,
>  		},
>  	};
>  
> @@ -291,9 +291,9 @@ static int adis16220_check_status(struct device *dev)
>  	if (status & ADIS16220_DIAG_STAT_FLASH_UPT)
>  		dev_err(dev, "Flash update failed\n");
>  	if (status & ADIS16220_DIAG_STAT_POWER_HIGH)
> -		dev_err(dev, "Power supply above 5.25V\n");
> +		dev_err(dev, "Power supply above 3.625V\n");
>  	if (status & ADIS16220_DIAG_STAT_POWER_LOW)
> -		dev_err(dev, "Power supply below 4.75V\n");
> +		dev_err(dev, "Power supply below 3.15V\n");
>  
>  error_ret:
>  	return ret;
> diff --git a/drivers/staging/iio/accel/adis16240_core.c b/drivers/staging/iio/accel/adis16240_core.c
> index 54fd6d7..731bca6 100644
> --- a/drivers/staging/iio/accel/adis16240_core.c
> +++ b/drivers/staging/iio/accel/adis16240_core.c
> @@ -74,13 +74,13 @@ static int adis16240_spi_write_reg_16(struct device *dev,
>  			.bits_per_word = 8,
>  			.len = 2,
>  			.cs_change = 1,
> -			.delay_usecs = 25,
> +			.delay_usecs = 35,
>  		}, {
>  			.tx_buf = st->tx + 2,
>  			.bits_per_word = 8,
>  			.len = 2,
>  			.cs_change = 1,
> -			.delay_usecs = 25,
> +			.delay_usecs = 35,
>  		},
>  	};
>  
> @@ -120,13 +120,13 @@ static int adis16240_spi_read_reg_16(struct device *dev,
>  			.bits_per_word = 8,
>  			.len = 2,
>  			.cs_change = 1,
> -			.delay_usecs = 25,
> +			.delay_usecs = 35,
>  		}, {
>  			.rx_buf = st->rx,
>  			.bits_per_word = 8,
>  			.len = 2,
>  			.cs_change = 1,
> -			.delay_usecs = 25,
> +			.delay_usecs = 35,
>  		},
>  	};
>  
> diff --git a/drivers/staging/iio/imu/adis16350_core.c b/drivers/staging/iio/imu/adis16350_core.c
> index 0edde73..0bb19a9 100644
> --- a/drivers/staging/iio/imu/adis16350_core.c
> +++ b/drivers/staging/iio/imu/adis16350_core.c
> @@ -75,13 +75,13 @@ static int adis16350_spi_write_reg_16(struct device *dev,
>  			.bits_per_word = 8,
>  			.len = 2,
>  			.cs_change = 1,
> -			.delay_usecs = 25,
> +			.delay_usecs = 35,
>  		}, {
>  			.tx_buf = st->tx + 2,
>  			.bits_per_word = 8,
>  			.len = 2,
>  			.cs_change = 1,
> -			.delay_usecs = 25,
> +			.delay_usecs = 35,
>  		},
>  	};
>  
> @@ -121,13 +121,13 @@ static int adis16350_spi_read_reg_16(struct device *dev,
>  			.bits_per_word = 8,
>  			.len = 2,
>  			.cs_change = 1,
> -			.delay_usecs = 25,
> +			.delay_usecs = 35,
>  		}, {
>  			.rx_buf = st->rx,
>  			.bits_per_word = 8,
>  			.len = 2,
>  			.cs_change = 1,
> -			.delay_usecs = 25,
> +			.delay_usecs = 35,
>  		},
>  	};
>  

Re: [PATCH 2/3] adis16400: fix some minor issues and clean-up

[Jonathan Cameron]

On 06/04/10 10:19, Barry Song wrote:
> 1. move adis16400_spi_read_burst() to adis16400_ring.c since it is only
>    called there
> 2. add the lost calling to adis16400_self_test()
> 3. codes cleanup
> 
Same issue with commit comment location
> Signed-off-by: Barry Song <21cnbao@gmail.com>
Acked-by: Jonathan Cameron <jic23@cam.ac.uk>
> ---
>  drivers/staging/iio/imu/adis16400.h      |    6 --
>  drivers/staging/iio/imu/adis16400_core.c |   84 +++++++----------------------
>  drivers/staging/iio/imu/adis16400_ring.c |   48 +++++++++++++++++
>  3 files changed, 70 insertions(+), 71 deletions(-)
...

Re: [PATCH 2/3] adis16400: fix some minor issues and clean-up

[Greg KH]

On Fri, Jun 04, 2010 at 11:58:14AM +0100, Jonathan Cameron wrote:
> On 06/04/10 10:19, Barry Song wrote:
> > 1. move adis16400_spi_read_burst() to adis16400_ring.c since it is only
> >    called there
> > 2. add the lost calling to adis16400_self_test()
> > 3. codes cleanup
> > 
> Same issue with commit comment location

Same confusion on my part, what's wrong with this?  This is what should
be in the changelog, right?

thanks,

greg k-h

Re: [PATCH 2/3] adis16400: fix some minor issues and clean-up

[Jonathan Cameron]

On 06/04/10 16:28, Greg KH wrote:
> On Fri, Jun 04, 2010 at 11:58:14AM +0100, Jonathan Cameron wrote:
>> On 06/04/10 10:19, Barry Song wrote:
>>> 1. move adis16400_spi_read_burst() to adis16400_ring.c since it is only
>>>    called there
>>> 2. add the lost calling to adis16400_self_test()
>>> 3. codes cleanup
>>>
>> Same issue with commit comment location
> 
> Same confusion on my part, what's wrong with this?  This is what should
> be in the changelog, right?
Fair enough. I just thought the clean up etc was pretty much covered by the title
and the rest was merely clarification. I'll go with your opinion though as you have
been doing this a lot longer than I have!

Barry, sorry for the confusion!

Jonathan

Re: [PATCH 1/3] adis16300: fix some minor issues and clean-up

[Jonathan Cameron]

On 06/04/10 10:19, Barry Song wrote:
> 1. add delay between spi transfers
> 2. move burst read to ring function
> 3. clean-up
I think I'm right in saying that, this lot will appear in the commit message.
Comments like this should go below.

Otherwise, looks fine to me.
> 
> Signed-off-by: Barry Song <21cnbao@gmail.com>
Acked-by: Jonathan Cameron <jic23@cam.ac.uk>
> ---
Comments here.
>  drivers/staging/iio/imu/adis16300.h      |    6 -
>  drivers/staging/iio/imu/adis16300_core.c |  151 +++++++++++++-----------------
>  drivers/staging/iio/imu/adis16300_ring.c |   54 ++++++++++-
>  3 files changed, 119 insertions(+), 95 deletions(-)
> 
> diff --git a/drivers/staging/iio/imu/adis16300.h b/drivers/staging/iio/imu/adis16300.h
> index 1c7ea5c..b050067 100644
> --- a/drivers/staging/iio/imu/adis16300.h
> +++ b/drivers/staging/iio/imu/adis16300.h
> @@ -115,14 +115,8 @@ struct adis16300_state {
>  	struct mutex			buf_lock;
>  };
>  
> -int adis16300_spi_read_burst(struct device *dev, u8 *rx);
> -
>  int adis16300_set_irq(struct device *dev, bool enable);
>  
> -int adis16300_reset(struct device *dev);
> -
> -int adis16300_check_status(struct device *dev);
> -
>  #ifdef CONFIG_IIO_RING_BUFFER
>  /* At the moment triggers are only used for ring buffer
>   * filling. This may change!
> diff --git a/drivers/staging/iio/imu/adis16300_core.c b/drivers/staging/iio/imu/adis16300_core.c
> index 5a7e5ef..28667e8 100644
> --- a/drivers/staging/iio/imu/adis16300_core.c
> +++ b/drivers/staging/iio/imu/adis16300_core.c
> @@ -29,10 +29,7 @@
>  
>  #define DRIVER_NAME		"adis16300"
>  
> -/* At the moment the spi framework doesn't allow global setting of cs_change.
> - * It's in the likely to be added comment at the top of spi.h.
> - * This means that use cannot be made of spi_write etc.
> - */
> +static int adis16300_check_status(struct device *dev);
>  
>  /**
>   * adis16300_spi_write_reg_8() - write single byte to a register
> @@ -79,11 +76,13 @@ static int adis16300_spi_write_reg_16(struct device *dev,
>  			.bits_per_word = 8,
>  			.len = 2,
>  			.cs_change = 1,
> +			.delay_usecs = 75,
>  		}, {
>  			.tx_buf = st->tx + 2,
>  			.bits_per_word = 8,
>  			.len = 2,
>  			.cs_change = 1,
> +			.delay_usecs = 75,
>  		},
>  	};
>  
> @@ -122,12 +121,14 @@ static int adis16300_spi_read_reg_16(struct device *dev,
>  			.tx_buf = st->tx,
>  			.bits_per_word = 8,
>  			.len = 2,
> -			.cs_change = 0,
> +			.cs_change = 1,
> +			.delay_usecs = 75,
>  		}, {
>  			.rx_buf = st->rx,
>  			.bits_per_word = 8,
>  			.len = 2,
> -			.cs_change = 0,
> +			.cs_change = 1,
> +			.delay_usecs = 75,
>  		},
>  	};
>  
> @@ -154,54 +155,6 @@ error_ret:
>  	return ret;
>  }
>  
> -/**
> - * adis16300_spi_read_burst() - read all data registers
> - * @dev: device associated with child of actual device (iio_dev or iio_trig)
> - * @rx: somewhere to pass back the value read (min size is 24 bytes)
> - **/
> -int adis16300_spi_read_burst(struct device *dev, u8 *rx)
> -{
> -	struct spi_message msg;
> -	struct iio_dev *indio_dev = dev_get_drvdata(dev);
> -	struct adis16300_state *st = iio_dev_get_devdata(indio_dev);
> -	u32 old_speed_hz = st->us->max_speed_hz;
> -	int ret;
> -
> -	struct spi_transfer xfers[] = {
> -		{
> -			.tx_buf = st->tx,
> -			.bits_per_word = 8,
> -			.len = 2,
> -			.cs_change = 0,
> -		}, {
> -			.rx_buf = rx,
> -			.bits_per_word = 8,
> -			.len = 18,
> -			.cs_change = 0,
> -		},
> -	};
> -
> -	mutex_lock(&st->buf_lock);
> -	st->tx[0] = ADIS16300_READ_REG(ADIS16300_GLOB_CMD);
> -	st->tx[1] = 0;
> -
> -	spi_message_init(&msg);
> -	spi_message_add_tail(&xfers[0], &msg);
> -	spi_message_add_tail(&xfers[1], &msg);
> -
> -	st->us->max_speed_hz = min(ADIS16300_SPI_BURST, old_speed_hz);
> -	spi_setup(st->us);
> -
> -	ret = spi_sync(st->us, &msg);
> -	if (ret)
> -		dev_err(&st->us->dev, "problem when burst reading");
> -
> -	st->us->max_speed_hz = old_speed_hz;
> -	spi_setup(st->us);
> -	mutex_unlock(&st->buf_lock);
> -	return ret;
> -}
> -
>  static ssize_t adis16300_spi_read_signed(struct device *dev,
>  		struct device_attribute *attr,
>  		char *buf,
> @@ -240,6 +193,24 @@ static ssize_t adis16300_read_12bit_unsigned(struct device *dev,
>  	return sprintf(buf, "%u\n", val & 0x0FFF);
>  }
>  
> +static ssize_t adis16300_read_14bit_unsigned(struct device *dev,
> +		struct device_attribute *attr,
> +		char *buf)
> +{
> +	int ret;
> +	u16 val = 0;
> +	struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
> +
> +	ret = adis16300_spi_read_reg_16(dev, this_attr->address, &val);
> +	if (ret)
> +		return ret;
> +
> +	if (val & ADIS16300_ERROR_ACTIVE)
> +		adis16300_check_status(dev);
> +
> +	return sprintf(buf, "%u\n", val & 0x3FFF);
> +}
> +
>  static ssize_t adis16300_read_14bit_signed(struct device *dev,
>  		struct device_attribute *attr,
>  		char *buf)
> @@ -356,6 +327,18 @@ static ssize_t adis16300_write_frequency(struct device *dev,
>  	return ret ? ret : len;
>  }
>  
> +static int adis16300_reset(struct device *dev)
> +{
> +	int ret;
> +	ret = adis16300_spi_write_reg_8(dev,
> +			ADIS16300_GLOB_CMD,
> +			ADIS16300_GLOB_CMD_SW_RESET);
> +	if (ret)
> +		dev_err(dev, "problem resetting device");
> +
> +	return ret;
> +}
> +
>  static ssize_t adis16300_write_reset(struct device *dev,
>  		struct device_attribute *attr,
>  		const char *buf, size_t len)
> @@ -371,8 +354,6 @@ static ssize_t adis16300_write_reset(struct device *dev,
>  	return -1;
>  }
>  
> -
> -
>  int adis16300_set_irq(struct device *dev, bool enable)
>  {
>  	int ret;
> @@ -396,32 +377,37 @@ error_ret:
>  	return ret;
>  }
>  
> -int adis16300_reset(struct device *dev)
> +/* Power down the device */
> +static int adis16300_stop_device(struct device *dev)
>  {
>  	int ret;
> -	ret = adis16300_spi_write_reg_8(dev,
> -			ADIS16300_GLOB_CMD,
> -			ADIS16300_GLOB_CMD_SW_RESET);
> +	u16 val = ADIS16300_SLP_CNT_POWER_OFF;
> +
> +	ret = adis16300_spi_write_reg_16(dev, ADIS16300_SLP_CNT, val);
>  	if (ret)
> -		dev_err(dev, "problem resetting device");
> +		dev_err(dev, "problem with turning device off: SLP_CNT");
>  
>  	return ret;
>  }
>  
> -/* Power down the device */
> -static int adis16300_stop_device(struct device *dev)
> +static int adis16300_self_test(struct device *dev)
>  {
>  	int ret;
> -	u16 val = ADIS16300_SLP_CNT_POWER_OFF;
> +	ret = adis16300_spi_write_reg_16(dev,
> +			ADIS16300_MSC_CTRL,
> +			ADIS16300_MSC_CTRL_MEM_TEST);
> +	if (ret) {
> +		dev_err(dev, "problem starting self test");
> +		goto err_ret;
> +	}
>  
> -	ret = adis16300_spi_write_reg_16(dev, ADIS16300_SLP_CNT, val);
> -	if (ret)
> -		dev_err(dev, "problem with turning device off: SLP_CNT");
> +	adis16300_check_status(dev);
>  
> +err_ret:
>  	return ret;
>  }
>  
> -int adis16300_check_status(struct device *dev)
> +static int adis16300_check_status(struct device *dev)
>  {
>  	u16 status;
>  	int ret;
> @@ -483,6 +469,11 @@ static int adis16300_initial_setup(struct adis16300_state *st)
>  	}
>  
>  	/* Do self test */
> +	ret = adis16300_self_test(dev);
> +	if (ret) {
> +		dev_err(dev, "self test failure");
> +		goto err_ret;
> +	}
>  
>  	/* Read status register to check the result */
>  	ret = adis16300_check_status(dev);
> @@ -526,7 +517,7 @@ static IIO_DEV_ATTR_ACCEL_Z_OFFSET(S_IWUSR | S_IRUGO,
>  		adis16300_write_16bit,
>  		ADIS16300_ZACCL_OFF);
>  
> -static IIO_DEV_ATTR_IN_NAMED_RAW(supply, adis16300_read_14bit_signed,
> +static IIO_DEV_ATTR_IN_NAMED_RAW(supply, adis16300_read_14bit_unsigned,
>  			   ADIS16300_SUPPLY_OUT);
>  static IIO_CONST_ATTR(in_supply_scale, "0.00242");
>  
> @@ -548,7 +539,7 @@ static IIO_DEV_ATTR_INCLI_Y(adis16300_read_13bit_signed,
>  		ADIS16300_YINCLI_OUT);
>  static IIO_CONST_ATTR(incli_scale, "0.044 d");
>  
> -static IIO_DEV_ATTR_TEMP_RAW(adis16300_read_12bit_signed);
> +static IIO_DEV_ATTR_TEMP_RAW(adis16300_read_12bit_unsigned);
>  static IIO_CONST_ATTR(temp_offset, "198.16 K");
>  static IIO_CONST_ATTR(temp_scale, "0.14 K");
>  
> @@ -659,15 +650,7 @@ static int __devinit adis16300_probe(struct spi_device *spi)
>  		goto error_unreg_ring_funcs;
>  	}
>  
> -	if (spi->irq && gpio_is_valid(irq_to_gpio(spi->irq)) > 0) {
> -#if 0 /* fixme: here we should support */
> -		iio_init_work_cont(&st->work_cont_thresh,
> -				NULL,
> -				adis16300_thresh_handler_bh_no_check,
> -				0,
> -				0,
> -				st);
> -#endif
> +	if (spi->irq) {
>  		ret = iio_register_interrupt_line(spi->irq,
>  				st->indio_dev,
>  				0,
> @@ -688,10 +671,9 @@ static int __devinit adis16300_probe(struct spi_device *spi)
>  	return 0;
>  
>  error_remove_trigger:
> -	if (st->indio_dev->modes & INDIO_RING_TRIGGERED)
> -		adis16300_remove_trigger(st->indio_dev);
> +	adis16300_remove_trigger(st->indio_dev);
>  error_unregister_line:
> -	if (st->indio_dev->modes & INDIO_RING_TRIGGERED)
> +	if (spi->irq)
>  		iio_unregister_interrupt_line(st->indio_dev, 0);
>  error_uninitialize_ring:
>  	adis16300_uninitialize_ring(st->indio_dev->ring);
> @@ -712,7 +694,6 @@ error_ret:
>  	return ret;
>  }
>  
> -/* fixme, confirm ordering in this function */
>  static int adis16300_remove(struct spi_device *spi)
>  {
>  	int ret;
> @@ -726,12 +707,12 @@ static int adis16300_remove(struct spi_device *spi)
>  	flush_scheduled_work();
>  
>  	adis16300_remove_trigger(indio_dev);
> -	if (spi->irq && gpio_is_valid(irq_to_gpio(spi->irq)) > 0)
> +	if (spi->irq)
>  		iio_unregister_interrupt_line(indio_dev, 0);
>  
>  	adis16300_uninitialize_ring(indio_dev->ring);
> -	adis16300_unconfigure_ring(indio_dev);
>  	iio_device_unregister(indio_dev);
> +	adis16300_unconfigure_ring(indio_dev);
>  	kfree(st->tx);
>  	kfree(st->rx);
>  	kfree(st);
> diff --git a/drivers/staging/iio/imu/adis16300_ring.c b/drivers/staging/iio/imu/adis16300_ring.c
> index 76cf8a6..17ceb72 100644
> --- a/drivers/staging/iio/imu/adis16300_ring.c
> +++ b/drivers/staging/iio/imu/adis16300_ring.c
> @@ -26,7 +27,7 @@ static inline u16 combine_8_to_16(u8 lower, u8 upper)
>  	return _lower | (_upper << 8);
>  }
>  
> -static IIO_SCAN_EL_C(supply, ADIS16300_SCAN_SUPPLY, IIO_SIGNED(14),
> +static IIO_SCAN_EL_C(supply, ADIS16300_SCAN_SUPPLY, IIO_UNSIGNED(14),
>  		     ADIS16300_SUPPLY_OUT, NULL);
>  
>  static IIO_SCAN_EL_C(gyro_x, ADIS16300_SCAN_GYRO_X, IIO_SIGNED(14),
> @@ -39,9 +40,9 @@ static IIO_SCAN_EL_C(accel_y, ADIS16300_SCAN_ACC_Y, IIO_SIGNED(14),
>  static IIO_SCAN_EL_C(accel_z, ADIS16300_SCAN_ACC_Z, IIO_SIGNED(14),
>  		     ADIS16300_ZACCL_OUT, NULL);
>  
> -static IIO_SCAN_EL_C(temp, ADIS16300_SCAN_TEMP, IIO_SIGNED(12),
> +static IIO_SCAN_EL_C(temp, ADIS16300_SCAN_TEMP, IIO_UNSIGNED(12),
>  		     ADIS16300_TEMP_OUT, NULL);
> -static IIO_SCAN_EL_C(adc_0, ADIS16300_SCAN_ADC_0, IIO_SIGNED(12),
> +static IIO_SCAN_EL_C(adc_0, ADIS16300_SCAN_ADC_0, IIO_UNSIGNED(12),
>  		     ADIS16300_AUX_ADC, NULL);
>  
>  static IIO_SCAN_EL_C(incli_x, ADIS16300_SCAN_INCLI_X, IIO_SIGNED(12),
> @@ -87,6 +88,54 @@ static void adis16300_poll_func_th(struct iio_dev *indio_dev)
>  	 */
>  }
>  
> +/**
> + * adis16300_spi_read_burst() - read all data registers
> + * @dev: device associated with child of actual device (iio_dev or iio_trig)
> + * @rx: somewhere to pass back the value read (min size is 24 bytes)
> + **/
> +static int adis16300_spi_read_burst(struct device *dev, u8 *rx)
> +{
> +	struct spi_message msg;
> +	struct iio_dev *indio_dev = dev_get_drvdata(dev);
> +	struct adis16300_state *st = iio_dev_get_devdata(indio_dev);
> +	u32 old_speed_hz = st->us->max_speed_hz;
> +	int ret;
> +
> +	struct spi_transfer xfers[] = {
> +		{
> +			.tx_buf = st->tx,
> +			.bits_per_word = 8,
> +			.len = 2,
> +			.cs_change = 0,
> +		}, {
> +			.rx_buf = rx,
> +			.bits_per_word = 8,
> +			.len = 18,
> +			.cs_change = 0,
> +		},
> +	};
> +
> +	mutex_lock(&st->buf_lock);
> +	st->tx[0] = ADIS16300_READ_REG(ADIS16300_GLOB_CMD);
> +	st->tx[1] = 0;
> +
> +	spi_message_init(&msg);
> +	spi_message_add_tail(&xfers[0], &msg);
> +	spi_message_add_tail(&xfers[1], &msg);
> +
> +	st->us->max_speed_hz = ADIS16300_SPI_BURST;
> +	spi_setup(st->us);
> +
> +	ret = spi_sync(st->us, &msg);
> +	if (ret)
> +		dev_err(&st->us->dev, "problem when burst reading");
> +
> +	st->us->max_speed_hz = old_speed_hz;
> +	spi_setup(st->us);
> +	mutex_unlock(&st->buf_lock);
> +	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.
>   */

Re: [PATCH 1/3] adis16300: fix some minor issues and clean-up

[Greg KH]

On Fri, Jun 04, 2010 at 11:56:05AM +0100, Jonathan Cameron wrote:
> On 06/04/10 10:19, Barry Song wrote:
> > 1. add delay between spi transfers
> > 2. move burst read to ring function
> > 3. clean-up
> I think I'm right in saying that, this lot will appear in the commit message.
> Comments like this should go below.

Yes, and that's good, it should be in the commit message, right?

thanks,

greg k-h

Re: [PATCH 1/3] adis16300: fix some minor issues and clean-up

[Barry Song]

On Fri, Jun 4, 2010 at 11:28 PM, Greg KH <gregkh@suse.de> wrote:
> On Fri, Jun 04, 2010 at 11:56:05AM +0100, Jonathan Cameron wrote:
>> On 06/04/10 10:19, Barry Song wrote:
>> > 1. add delay between spi transfers
>> > 2. move burst read to ring function
>> > 3. clean-up
>> I think I'm right in saying that, this lot will appear in the commit message.
>> Comments like this should go below.
>
> Yes, and that's good, it should be in the commit message, right?
Yes. it should be in the commit message, then people know the details
about this commit.
>
> thanks,
>
> greg k-h
>