All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/5] staging:iio:ad7298: Do not perform endianness conversion in buffered mode
@ 2012-11-15 13:15 Lars-Peter Clausen
  2012-11-15 13:15 ` [PATCH 2/5] staging:iio:ad7298: Rework regulator handling Lars-Peter Clausen
                   ` (4 more replies)
  0 siblings, 5 replies; 12+ messages in thread
From: Lars-Peter Clausen @ 2012-11-15 13:15 UTC (permalink / raw)
  To: Jonathan Cameron; +Cc: linux-iio, drivers, Lars-Peter Clausen

For buffered mode we do not want to perform endianness conversion in the kernel,
but rather offload it to user space, since it is not always required to do a
conversion at all. It also greatly simplifies the kernel code since no
post-processing has to be done and may allow future optimizations like streaming
data directly to a storage device or over the network via DMA.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/staging/iio/adc/ad7298.h      |  2 +-
 drivers/staging/iio/adc/ad7298_core.c |  1 +
 drivers/staging/iio/adc/ad7298_ring.c | 11 +++--------
 3 files changed, 5 insertions(+), 9 deletions(-)

diff --git a/drivers/staging/iio/adc/ad7298.h b/drivers/staging/iio/adc/ad7298.h
index 18f2787..0ce9031 100644
--- a/drivers/staging/iio/adc/ad7298.h
+++ b/drivers/staging/iio/adc/ad7298.h
@@ -48,7 +48,7 @@ struct ad7298_state {
 	 * DMA (thus cache coherency maintenance) requires the
 	 * transfer buffers to live in their own cache lines.
 	 */
-	unsigned short			rx_buf[8] ____cacheline_aligned;
+	unsigned short			rx_buf[12] ____cacheline_aligned;
 	unsigned short			tx_buf[2];
 };
 
diff --git a/drivers/staging/iio/adc/ad7298_core.c b/drivers/staging/iio/adc/ad7298_core.c
index 4c75114..67082ad 100644
--- a/drivers/staging/iio/adc/ad7298_core.c
+++ b/drivers/staging/iio/adc/ad7298_core.c
@@ -35,6 +35,7 @@
 			.sign = 'u',					\
 			.realbits = 12,					\
 			.storagebits = 16,				\
+			.endianness = IIO_BE,				\
 		},							\
 	}
 
diff --git a/drivers/staging/iio/adc/ad7298_ring.c b/drivers/staging/iio/adc/ad7298_ring.c
index b3dd514..e387712 100644
--- a/drivers/staging/iio/adc/ad7298_ring.c
+++ b/drivers/staging/iio/adc/ad7298_ring.c
@@ -76,8 +76,7 @@ static irqreturn_t ad7298_trigger_handler(int irq, void *p)
 	struct iio_dev *indio_dev = pf->indio_dev;
 	struct ad7298_state *st = iio_priv(indio_dev);
 	s64 time_ns = 0;
-	__u16 buf[16];
-	int b_sent, i;
+	int b_sent;
 
 	b_sent = spi_sync(st->spi, &st->ring_msg);
 	if (b_sent)
@@ -85,15 +84,11 @@ static irqreturn_t ad7298_trigger_handler(int irq, void *p)
 
 	if (indio_dev->scan_timestamp) {
 		time_ns = iio_get_time_ns();
-		memcpy((u8 *)buf + indio_dev->scan_bytes - sizeof(s64),
+		memcpy((u8 *)st->rx_buf + indio_dev->scan_bytes - sizeof(s64),
 			&time_ns, sizeof(time_ns));
 	}
 
-	for (i = 0; i < bitmap_weight(indio_dev->active_scan_mask,
-						 indio_dev->masklength); i++)
-		buf[i] = be16_to_cpu(st->rx_buf[i]);
-
-	iio_push_to_buffers(indio_dev, (u8 *)buf);
+	iio_push_to_buffers(indio_dev, (u8 *)st->rx_buf);
 
 done:
 	iio_trigger_notify_done(indio_dev->trig);
-- 
1.8.0

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/5] staging:iio:ad7298: Rework regulator handling
  2012-11-15 13:15 [PATCH 1/5] staging:iio:ad7298: Do not perform endianness conversion in buffered mode Lars-Peter Clausen
@ 2012-11-15 13:15 ` Lars-Peter Clausen
  2012-11-17 11:17   ` Jonathan Cameron
  2012-11-15 13:15 ` [PATCH 3/5] staging:iio:ad7298: Fix temperature scale and offset Lars-Peter Clausen
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 12+ messages in thread
From: Lars-Peter Clausen @ 2012-11-15 13:15 UTC (permalink / raw)
  To: Jonathan Cameron; +Cc: linux-iio, drivers, Lars-Peter Clausen

Rework the regulator handling of the driver to match more closely what we do in
other drivers. Make the regulator non-optional if a external reference is used.
Also dispose the option of specifying the reference voltage via platform data.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/staging/iio/adc/ad7298.h      | 12 ++++------
 drivers/staging/iio/adc/ad7298_core.c | 44 +++++++++++++++++++++--------------
 2 files changed, 31 insertions(+), 25 deletions(-)

diff --git a/drivers/staging/iio/adc/ad7298.h b/drivers/staging/iio/adc/ad7298.h
index 0ce9031..6523e01 100644
--- a/drivers/staging/iio/adc/ad7298.h
+++ b/drivers/staging/iio/adc/ad7298.h
@@ -26,19 +26,17 @@
 
 #define RES_MASK(bits)	((1 << (bits)) - 1)
 
-/*
- * TODO: struct ad7298_platform_data needs to go into include/linux/iio
- */
-
+/**
+ * struct ad7298_platform_data - Platform data for the ad7298 ADC driver
+ * @ext_ref: Whether to use an external reference voltage.
+ **/
 struct ad7298_platform_data {
-	/* External Vref voltage applied */
-	u16				vref_mv;
+	bool ext_ref;
 };
 
 struct ad7298_state {
 	struct spi_device		*spi;
 	struct regulator		*reg;
-	u16				int_vref_mv;
 	unsigned			ext_ref;
 	struct spi_transfer		ring_xfer[10];
 	struct spi_transfer		scan_single_xfer[3];
diff --git a/drivers/staging/iio/adc/ad7298_core.c b/drivers/staging/iio/adc/ad7298_core.c
index 67082ad..6dd696f 100644
--- a/drivers/staging/iio/adc/ad7298_core.c
+++ b/drivers/staging/iio/adc/ad7298_core.c
@@ -130,7 +130,7 @@ static int ad7298_read_raw(struct iio_dev *indio_dev,
 {
 	int ret;
 	struct ad7298_state *st = iio_priv(indio_dev);
-	unsigned int scale_uv;
+	int scale_mv;
 
 	switch (m) {
 	case IIO_CHAN_INFO_RAW:
@@ -155,10 +155,17 @@ static int ad7298_read_raw(struct iio_dev *indio_dev,
 	case IIO_CHAN_INFO_SCALE:
 		switch (chan->type) {
 		case IIO_VOLTAGE:
-			scale_uv = (st->int_vref_mv * 1000) >> AD7298_BITS;
-			*val =  scale_uv / 1000;
-			*val2 = (scale_uv % 1000) * 1000;
-			return IIO_VAL_INT_PLUS_MICRO;
+			if (st->ext_ref) {
+				scale_mv = regulator_get_voltage(st->reg);
+				if (scale_mv < 0)
+					return scale_mv;
+				scale_mv /= 1000;
+			} else {
+				scale_mv = AD7298_INTREF_mV;
+			}
+			*val =  scale_mv;
+			*val2 = chan->scan_type.realbits;
+			return IIO_VAL_FRACTIONAL_LOG2;
 		case IIO_TEMP:
 			*val =  1;
 			*val2 = 0;
@@ -180,16 +187,23 @@ static int __devinit ad7298_probe(struct spi_device *spi)
 {
 	struct ad7298_platform_data *pdata = spi->dev.platform_data;
 	struct ad7298_state *st;
-	int ret;
 	struct iio_dev *indio_dev = iio_device_alloc(sizeof(*st));
+	int ret;
 
 	if (indio_dev == NULL)
 		return -ENOMEM;
 
 	st = iio_priv(indio_dev);
 
-	st->reg = regulator_get(&spi->dev, "vcc");
-	if (!IS_ERR(st->reg)) {
+	if (pdata && pdata->ext_ref)
+		st->ext_ref = AD7298_EXTREF;
+
+	if (st->ext_ref) {
+		st->reg = regulator_get(&spi->dev, "vref");
+		if (IS_ERR(st->reg)) {
+			ret = PTR_ERR(st->reg);
+			goto error_free;
+		}
 		ret = regulator_enable(st->reg);
 		if (ret)
 			goto error_put_reg;
@@ -222,13 +236,6 @@ static int __devinit ad7298_probe(struct spi_device *spi)
 	spi_message_add_tail(&st->scan_single_xfer[1], &st->scan_single_msg);
 	spi_message_add_tail(&st->scan_single_xfer[2], &st->scan_single_msg);
 
-	if (pdata && pdata->vref_mv) {
-		st->int_vref_mv = pdata->vref_mv;
-		st->ext_ref = AD7298_EXTREF;
-	} else {
-		st->int_vref_mv = AD7298_INTREF_mV;
-	}
-
 	ret = ad7298_register_ring_funcs_and_init(indio_dev);
 	if (ret)
 		goto error_disable_reg;
@@ -242,11 +249,12 @@ static int __devinit ad7298_probe(struct spi_device *spi)
 error_cleanup_ring:
 	ad7298_ring_cleanup(indio_dev);
 error_disable_reg:
-	if (!IS_ERR(st->reg))
+	if (st->ext_ref)
 		regulator_disable(st->reg);
 error_put_reg:
-	if (!IS_ERR(st->reg))
+	if (st->ext_ref)
 		regulator_put(st->reg);
+error_free:
 	iio_device_free(indio_dev);
 
 	return ret;
@@ -259,7 +267,7 @@ static int __devexit ad7298_remove(struct spi_device *spi)
 
 	iio_device_unregister(indio_dev);
 	ad7298_ring_cleanup(indio_dev);
-	if (!IS_ERR(st->reg)) {
+	if (st->ext_ref) {
 		regulator_disable(st->reg);
 		regulator_put(st->reg);
 	}
-- 
1.8.0

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 3/5] staging:iio:ad7298: Fix temperature scale and offset
  2012-11-15 13:15 [PATCH 1/5] staging:iio:ad7298: Do not perform endianness conversion in buffered mode Lars-Peter Clausen
  2012-11-15 13:15 ` [PATCH 2/5] staging:iio:ad7298: Rework regulator handling Lars-Peter Clausen
@ 2012-11-15 13:15 ` Lars-Peter Clausen
  2012-11-17 11:19   ` Jonathan Cameron
  2012-11-15 13:15 ` [PATCH 4/5] staging:iio:ad7298: Squash everything into one file Lars-Peter Clausen
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 12+ messages in thread
From: Lars-Peter Clausen @ 2012-11-15 13:15 UTC (permalink / raw)
  To: Jonathan Cameron; +Cc: linux-iio, drivers, Lars-Peter Clausen

The temperature scale and offset depend on the reference voltage, the current
formula used in the driver assumes a 2.5V reference. This patch modifies the
code to report the unprocessed value for the temperature channel "raw" property
and to provide proper "scale" and "offset" properties which depend on the
selected reference voltage.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/staging/iio/adc/ad7298_core.c | 51 ++++++++++++++++-------------------
 1 file changed, 23 insertions(+), 28 deletions(-)

diff --git a/drivers/staging/iio/adc/ad7298_core.c b/drivers/staging/iio/adc/ad7298_core.c
index 6dd696f..b74b76f 100644
--- a/drivers/staging/iio/adc/ad7298_core.c
+++ b/drivers/staging/iio/adc/ad7298_core.c
@@ -45,7 +45,8 @@ static const struct iio_chan_spec ad7298_channels[] = {
 		.indexed = 1,
 		.channel = 0,
 		.info_mask = IIO_CHAN_INFO_RAW_SEPARATE_BIT |
-		IIO_CHAN_INFO_SCALE_SEPARATE_BIT,
+			IIO_CHAN_INFO_SCALE_SEPARATE_BIT |
+			IIO_CHAN_INFO_OFFSET_SEPARATE_BIT,
 		.address = AD7298_CH_TEMP,
 		.scan_index = -1,
 		.scan_type = {
@@ -80,7 +81,7 @@ static int ad7298_scan_direct(struct ad7298_state *st, unsigned ch)
 
 static int ad7298_scan_temp(struct ad7298_state *st, int *val)
 {
-	int tmp, ret;
+	int ret;
 	__be16 buf;
 
 	buf = cpu_to_be16(AD7298_WRITE | AD7298_TSENSE |
@@ -102,24 +103,24 @@ static int ad7298_scan_temp(struct ad7298_state *st, int *val)
 	if (ret)
 		return ret;
 
-	tmp = be16_to_cpu(buf) & RES_MASK(AD7298_BITS);
+	*val = sign_extend32(be16_to_cpu(buf), 11);
 
-	/*
-	 * One LSB of the ADC corresponds to 0.25 deg C.
-	 * The temperature reading is in 12-bit twos complement format
-	 */
+	return 0;
+}
+
+static int ad7298_get_ref_voltage(struct ad7298_state *st)
+{
+	int vref;
 
-	if (tmp & (1 << (AD7298_BITS - 1))) {
-		tmp = (4096 - tmp) * 250;
-		tmp -= (2 * tmp);
+	if (st->ext_ref) {
+		vref = regulator_get_voltage(st->reg);
+		if (vref < 0)
+			return vref;
 
+		return vref / 1000;
 	} else {
-		tmp *= 250; /* temperature in milli degrees Celsius */
+		return AD7298_INTREF_mV;
 	}
-
-	*val = tmp;
-
-	return 0;
 }
 
 static int ad7298_read_raw(struct iio_dev *indio_dev,
@@ -130,7 +131,6 @@ static int ad7298_read_raw(struct iio_dev *indio_dev,
 {
 	int ret;
 	struct ad7298_state *st = iio_priv(indio_dev);
-	int scale_mv;
 
 	switch (m) {
 	case IIO_CHAN_INFO_RAW:
@@ -155,24 +155,19 @@ static int ad7298_read_raw(struct iio_dev *indio_dev,
 	case IIO_CHAN_INFO_SCALE:
 		switch (chan->type) {
 		case IIO_VOLTAGE:
-			if (st->ext_ref) {
-				scale_mv = regulator_get_voltage(st->reg);
-				if (scale_mv < 0)
-					return scale_mv;
-				scale_mv /= 1000;
-			} else {
-				scale_mv = AD7298_INTREF_mV;
-			}
-			*val =  scale_mv;
+			*val = ad7298_get_ref_voltage(st);
 			*val2 = chan->scan_type.realbits;
 			return IIO_VAL_FRACTIONAL_LOG2;
 		case IIO_TEMP:
-			*val =  1;
-			*val2 = 0;
-			return IIO_VAL_INT_PLUS_MICRO;
+			*val = ad7298_get_ref_voltage(st);
+			*val2 = 10;
+			return IIO_VAL_FRACTIONAL;
 		default:
 			return -EINVAL;
 		}
+	case IIO_CHAN_INFO_OFFSET:
+		*val = 1093 - 2732500 / ad7298_get_ref_voltage(st);
+		return IIO_VAL_INT;
 	}
 	return -EINVAL;
 }
-- 
1.8.0

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 4/5] staging:iio:ad7298: Squash everything into one file
  2012-11-15 13:15 [PATCH 1/5] staging:iio:ad7298: Do not perform endianness conversion in buffered mode Lars-Peter Clausen
  2012-11-15 13:15 ` [PATCH 2/5] staging:iio:ad7298: Rework regulator handling Lars-Peter Clausen
  2012-11-15 13:15 ` [PATCH 3/5] staging:iio:ad7298: Fix temperature scale and offset Lars-Peter Clausen
@ 2012-11-15 13:15 ` Lars-Peter Clausen
  2012-11-17 11:20   ` Jonathan Cameron
  2012-11-15 13:15 ` [PATCH 5/5] staging:iio: Move the ad7298 driver out of staging Lars-Peter Clausen
  2012-11-17 11:16 ` [PATCH 1/5] staging:iio:ad7298: Do not perform endianness conversion in buffered mode Jonathan Cameron
  4 siblings, 1 reply; 12+ messages in thread
From: Lars-Peter Clausen @ 2012-11-15 13:15 UTC (permalink / raw)
  To: Jonathan Cameron; +Cc: linux-iio, drivers, Lars-Peter Clausen

The recent cleanups have decimated the drivers code size by quite a bit. It is
only a few hundred lines in total now. Putting everything into one file also
allows to reduce the code size a bit more by removing a few lines of boilerplate
code. The only functional change made by this patch is that we now always
include buffer support, instead of making it optional. This is more consistent
with what we do for other drivers.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/staging/iio/adc/Kconfig                    |   3 +-
 drivers/staging/iio/adc/Makefile                   |   3 +-
 .../staging/iio/adc/{ad7298_core.c => ad7298.c}    | 121 ++++++++++++++++++++-
 drivers/staging/iio/adc/ad7298.h                   |  53 ---------
 drivers/staging/iio/adc/ad7298_ring.c              | 108 ------------------
 5 files changed, 121 insertions(+), 167 deletions(-)
 rename drivers/staging/iio/adc/{ad7298_core.c => ad7298.c} (64%)
 delete mode 100644 drivers/staging/iio/adc/ad7298_ring.c

diff --git a/drivers/staging/iio/adc/Kconfig b/drivers/staging/iio/adc/Kconfig
index 0177f1e..5086a46 100644
--- a/drivers/staging/iio/adc/Kconfig
+++ b/drivers/staging/iio/adc/Kconfig
@@ -13,7 +13,8 @@ config AD7291
 config AD7298
 	tristate "Analog Devices AD7298 ADC driver"
 	depends on SPI
-	select IIO_TRIGGERED_BUFFER if IIO_BUFFER
+	select IIO_BUFFER
+	select IIO_TRIGGERED_BUFFER
 	help
 	  Say yes here to build support for Analog Devices AD7298
 	  8 Channel ADC with temperature sensor.
diff --git a/drivers/staging/iio/adc/Makefile b/drivers/staging/iio/adc/Makefile
index 12b4bd3..e867d01 100644
--- a/drivers/staging/iio/adc/Makefile
+++ b/drivers/staging/iio/adc/Makefile
@@ -12,8 +12,7 @@ ad799x-y := ad799x_core.o
 ad799x-$(CONFIG_AD799X_RING_BUFFER) += ad799x_ring.o
 obj-$(CONFIG_AD799X) += ad799x.o
 
-ad7298-y := ad7298_core.o
-ad7298-$(CONFIG_IIO_BUFFER) += ad7298_ring.o
+obj-$(CONFIG_AD7887) += ad7887.o
 obj-$(CONFIG_AD7298) += ad7298.o
 
 obj-$(CONFIG_AD7291) += ad7291.o
diff --git a/drivers/staging/iio/adc/ad7298_core.c b/drivers/staging/iio/adc/ad7298.c
similarity index 64%
rename from drivers/staging/iio/adc/ad7298_core.c
rename to drivers/staging/iio/adc/ad7298.c
index b74b76f..2742a9d 100644
--- a/drivers/staging/iio/adc/ad7298_core.c
+++ b/drivers/staging/iio/adc/ad7298.c
@@ -15,13 +15,49 @@
 #include <linux/err.h>
 #include <linux/delay.h>
 #include <linux/module.h>
+#include <linux/interrupt.h>
 
 #include <linux/iio/iio.h>
 #include <linux/iio/sysfs.h>
 #include <linux/iio/buffer.h>
+#include <linux/iio/trigger_consumer.h>
+#include <linux/iio/triggered_buffer.h>
 
 #include "ad7298.h"
 
+#define AD7298_WRITE	(1 << 15) /* write to the control register */
+#define AD7298_REPEAT	(1 << 14) /* repeated conversion enable */
+#define AD7298_CH(x)	(1 << (13 - (x))) /* channel select */
+#define AD7298_TSENSE	(1 << 5) /* temperature conversion enable */
+#define AD7298_EXTREF	(1 << 2) /* external reference enable */
+#define AD7298_TAVG	(1 << 1) /* temperature sensor averaging enable */
+#define AD7298_PDD	(1 << 0) /* partial power down enable */
+
+#define AD7298_MAX_CHAN		8
+#define AD7298_BITS		12
+#define AD7298_STORAGE_BITS	16
+#define AD7298_INTREF_mV	2500
+
+#define AD7298_CH_TEMP		9
+
+#define RES_MASK(bits)	((1 << (bits)) - 1)
+
+struct ad7298_state {
+	struct spi_device		*spi;
+	struct regulator		*reg;
+	unsigned			ext_ref;
+	struct spi_transfer		ring_xfer[10];
+	struct spi_transfer		scan_single_xfer[3];
+	struct spi_message		ring_msg;
+	struct spi_message		scan_single_msg;
+	/*
+	 * DMA (thus cache coherency maintenance) requires the
+	 * transfer buffers to live in their own cache lines.
+	 */
+	unsigned short			rx_buf[12] ____cacheline_aligned;
+	unsigned short			tx_buf[2];
+};
+
 #define AD7298_V_CHAN(index)						\
 	{								\
 		.type = IIO_VOLTAGE,					\
@@ -66,6 +102,84 @@ static const struct iio_chan_spec ad7298_channels[] = {
 	IIO_CHAN_SOFT_TIMESTAMP(8),
 };
 
+/**
+ * ad7298_update_scan_mode() setup the spi transfer buffer for the new scan mask
+ **/
+static int ad7298_update_scan_mode(struct iio_dev *indio_dev,
+	const unsigned long *active_scan_mask)
+{
+	struct ad7298_state *st = iio_priv(indio_dev);
+	int i, m;
+	unsigned short command;
+	int scan_count;
+
+	/* Now compute overall size */
+	scan_count = bitmap_weight(active_scan_mask, indio_dev->masklength);
+
+	command = AD7298_WRITE | st->ext_ref;
+
+	for (i = 0, m = AD7298_CH(0); i < AD7298_MAX_CHAN; i++, m >>= 1)
+		if (test_bit(i, active_scan_mask))
+			command |= m;
+
+	st->tx_buf[0] = cpu_to_be16(command);
+
+	/* build spi ring message */
+	st->ring_xfer[0].tx_buf = &st->tx_buf[0];
+	st->ring_xfer[0].len = 2;
+	st->ring_xfer[0].cs_change = 1;
+	st->ring_xfer[1].tx_buf = &st->tx_buf[1];
+	st->ring_xfer[1].len = 2;
+	st->ring_xfer[1].cs_change = 1;
+
+	spi_message_init(&st->ring_msg);
+	spi_message_add_tail(&st->ring_xfer[0], &st->ring_msg);
+	spi_message_add_tail(&st->ring_xfer[1], &st->ring_msg);
+
+	for (i = 0; i < scan_count; i++) {
+		st->ring_xfer[i + 2].rx_buf = &st->rx_buf[i];
+		st->ring_xfer[i + 2].len = 2;
+		st->ring_xfer[i + 2].cs_change = 1;
+		spi_message_add_tail(&st->ring_xfer[i + 2], &st->ring_msg);
+	}
+	/* make sure last transfer cs_change is not set */
+	st->ring_xfer[i + 1].cs_change = 0;
+
+	return 0;
+}
+
+/**
+ * ad7298_trigger_handler() bh of trigger launched polling to ring buffer
+ *
+ * Currently there is no option in this driver to disable the saving of
+ * timestamps within the ring.
+ **/
+static irqreturn_t ad7298_trigger_handler(int irq, void *p)
+{
+	struct iio_poll_func *pf = p;
+	struct iio_dev *indio_dev = pf->indio_dev;
+	struct ad7298_state *st = iio_priv(indio_dev);
+	s64 time_ns = 0;
+	int b_sent;
+
+	b_sent = spi_sync(st->spi, &st->ring_msg);
+	if (b_sent)
+		goto done;
+
+	if (indio_dev->scan_timestamp) {
+		time_ns = iio_get_time_ns();
+		memcpy((u8 *)st->rx_buf + indio_dev->scan_bytes - sizeof(s64),
+			&time_ns, sizeof(time_ns));
+	}
+
+	iio_push_to_buffers(indio_dev, (u8 *)st->rx_buf);
+
+done:
+	iio_trigger_notify_done(indio_dev->trig);
+
+	return IRQ_HANDLED;
+}
+
 static int ad7298_scan_direct(struct ad7298_state *st, unsigned ch)
 {
 	int ret;
@@ -231,7 +345,8 @@ static int __devinit ad7298_probe(struct spi_device *spi)
 	spi_message_add_tail(&st->scan_single_xfer[1], &st->scan_single_msg);
 	spi_message_add_tail(&st->scan_single_xfer[2], &st->scan_single_msg);
 
-	ret = ad7298_register_ring_funcs_and_init(indio_dev);
+	ret = iio_triggered_buffer_setup(indio_dev, NULL,
+			&ad7298_trigger_handler, NULL);
 	if (ret)
 		goto error_disable_reg;
 
@@ -242,7 +357,7 @@ static int __devinit ad7298_probe(struct spi_device *spi)
 	return 0;
 
 error_cleanup_ring:
-	ad7298_ring_cleanup(indio_dev);
+	iio_triggered_buffer_cleanup(indio_dev);
 error_disable_reg:
 	if (st->ext_ref)
 		regulator_disable(st->reg);
@@ -261,7 +376,7 @@ static int __devexit ad7298_remove(struct spi_device *spi)
 	struct ad7298_state *st = iio_priv(indio_dev);
 
 	iio_device_unregister(indio_dev);
-	ad7298_ring_cleanup(indio_dev);
+	iio_triggered_buffer_cleanup(indio_dev);
 	if (st->ext_ref) {
 		regulator_disable(st->reg);
 		regulator_put(st->reg);
diff --git a/drivers/staging/iio/adc/ad7298.h b/drivers/staging/iio/adc/ad7298.h
index 6523e01..c8ac969 100644
--- a/drivers/staging/iio/adc/ad7298.h
+++ b/drivers/staging/iio/adc/ad7298.h
@@ -9,23 +9,6 @@
 #ifndef IIO_ADC_AD7298_H_
 #define IIO_ADC_AD7298_H_
 
-#define AD7298_WRITE	(1 << 15) /* write to the control register */
-#define AD7298_REPEAT	(1 << 14) /* repeated conversion enable */
-#define AD7298_CH(x)	(1 << (13 - (x))) /* channel select */
-#define AD7298_TSENSE	(1 << 5) /* temperature conversion enable */
-#define AD7298_EXTREF	(1 << 2) /* external reference enable */
-#define AD7298_TAVG	(1 << 1) /* temperature sensor averaging enable */
-#define AD7298_PDD	(1 << 0) /* partial power down enable */
-
-#define AD7298_MAX_CHAN		8
-#define AD7298_BITS		12
-#define AD7298_STORAGE_BITS	16
-#define AD7298_INTREF_mV	2500
-
-#define AD7298_CH_TEMP		9
-
-#define RES_MASK(bits)	((1 << (bits)) - 1)
-
 /**
  * struct ad7298_platform_data - Platform data for the ad7298 ADC driver
  * @ext_ref: Whether to use an external reference voltage.
@@ -34,40 +17,4 @@ struct ad7298_platform_data {
 	bool ext_ref;
 };
 
-struct ad7298_state {
-	struct spi_device		*spi;
-	struct regulator		*reg;
-	unsigned			ext_ref;
-	struct spi_transfer		ring_xfer[10];
-	struct spi_transfer		scan_single_xfer[3];
-	struct spi_message		ring_msg;
-	struct spi_message		scan_single_msg;
-	/*
-	 * DMA (thus cache coherency maintenance) requires the
-	 * transfer buffers to live in their own cache lines.
-	 */
-	unsigned short			rx_buf[12] ____cacheline_aligned;
-	unsigned short			tx_buf[2];
-};
-
-#ifdef CONFIG_IIO_BUFFER
-int ad7298_register_ring_funcs_and_init(struct iio_dev *indio_dev);
-void ad7298_ring_cleanup(struct iio_dev *indio_dev);
-int ad7298_update_scan_mode(struct iio_dev *indio_dev,
-	const unsigned long *active_scan_mask);
-#else /* CONFIG_IIO_BUFFER */
-
-static inline int
-ad7298_register_ring_funcs_and_init(struct iio_dev *indio_dev)
-{
-	return 0;
-}
-
-static inline void ad7298_ring_cleanup(struct iio_dev *indio_dev)
-{
-}
-
-#define ad7298_update_scan_mode NULL
-
-#endif /* CONFIG_IIO_BUFFER */
 #endif /* IIO_ADC_AD7298_H_ */
diff --git a/drivers/staging/iio/adc/ad7298_ring.c b/drivers/staging/iio/adc/ad7298_ring.c
deleted file mode 100644
index e387712..0000000
--- a/drivers/staging/iio/adc/ad7298_ring.c
+++ /dev/null
@@ -1,108 +0,0 @@
-/*
- * AD7298 SPI ADC driver
- *
- * Copyright 2011-2012 Analog Devices Inc.
- *
- * Licensed under the GPL-2.
- */
-
-#include <linux/interrupt.h>
-#include <linux/kernel.h>
-#include <linux/slab.h>
-#include <linux/spi/spi.h>
-
-#include <linux/iio/iio.h>
-#include <linux/iio/buffer.h>
-#include <linux/iio/trigger_consumer.h>
-#include <linux/iio/triggered_buffer.h>
-
-#include "ad7298.h"
-
-/**
- * ad7298_update_scan_mode() setup the spi transfer buffer for the new scan mask
- **/
-int ad7298_update_scan_mode(struct iio_dev *indio_dev,
-	const unsigned long *active_scan_mask)
-{
-	struct ad7298_state *st = iio_priv(indio_dev);
-	int i, m;
-	unsigned short command;
-	int scan_count;
-
-	/* Now compute overall size */
-	scan_count = bitmap_weight(active_scan_mask, indio_dev->masklength);
-
-	command = AD7298_WRITE | st->ext_ref;
-
-	for (i = 0, m = AD7298_CH(0); i < AD7298_MAX_CHAN; i++, m >>= 1)
-		if (test_bit(i, active_scan_mask))
-			command |= m;
-
-	st->tx_buf[0] = cpu_to_be16(command);
-
-	/* build spi ring message */
-	st->ring_xfer[0].tx_buf = &st->tx_buf[0];
-	st->ring_xfer[0].len = 2;
-	st->ring_xfer[0].cs_change = 1;
-	st->ring_xfer[1].tx_buf = &st->tx_buf[1];
-	st->ring_xfer[1].len = 2;
-	st->ring_xfer[1].cs_change = 1;
-
-	spi_message_init(&st->ring_msg);
-	spi_message_add_tail(&st->ring_xfer[0], &st->ring_msg);
-	spi_message_add_tail(&st->ring_xfer[1], &st->ring_msg);
-
-	for (i = 0; i < scan_count; i++) {
-		st->ring_xfer[i + 2].rx_buf = &st->rx_buf[i];
-		st->ring_xfer[i + 2].len = 2;
-		st->ring_xfer[i + 2].cs_change = 1;
-		spi_message_add_tail(&st->ring_xfer[i + 2], &st->ring_msg);
-	}
-	/* make sure last transfer cs_change is not set */
-	st->ring_xfer[i + 1].cs_change = 0;
-
-	return 0;
-}
-
-/**
- * ad7298_trigger_handler() bh of trigger launched polling to ring buffer
- *
- * Currently there is no option in this driver to disable the saving of
- * timestamps within the ring.
- **/
-static irqreturn_t ad7298_trigger_handler(int irq, void *p)
-{
-	struct iio_poll_func *pf = p;
-	struct iio_dev *indio_dev = pf->indio_dev;
-	struct ad7298_state *st = iio_priv(indio_dev);
-	s64 time_ns = 0;
-	int b_sent;
-
-	b_sent = spi_sync(st->spi, &st->ring_msg);
-	if (b_sent)
-		goto done;
-
-	if (indio_dev->scan_timestamp) {
-		time_ns = iio_get_time_ns();
-		memcpy((u8 *)st->rx_buf + indio_dev->scan_bytes - sizeof(s64),
-			&time_ns, sizeof(time_ns));
-	}
-
-	iio_push_to_buffers(indio_dev, (u8 *)st->rx_buf);
-
-done:
-	iio_trigger_notify_done(indio_dev->trig);
-
-	return IRQ_HANDLED;
-}
-
-int ad7298_register_ring_funcs_and_init(struct iio_dev *indio_dev)
-{
-	return iio_triggered_buffer_setup(indio_dev, NULL,
-			&ad7298_trigger_handler, NULL);
-}
-
-void ad7298_ring_cleanup(struct iio_dev *indio_dev)
-{
-	iio_triggered_buffer_cleanup(indio_dev);
-}
-- 
1.8.0

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 5/5] staging:iio: Move the ad7298 driver out of staging
  2012-11-15 13:15 [PATCH 1/5] staging:iio:ad7298: Do not perform endianness conversion in buffered mode Lars-Peter Clausen
                   ` (2 preceding siblings ...)
  2012-11-15 13:15 ` [PATCH 4/5] staging:iio:ad7298: Squash everything into one file Lars-Peter Clausen
@ 2012-11-15 13:15 ` Lars-Peter Clausen
  2012-11-17 11:45   ` Jonathan Cameron
  2012-11-17 11:16 ` [PATCH 1/5] staging:iio:ad7298: Do not perform endianness conversion in buffered mode Jonathan Cameron
  4 siblings, 1 reply; 12+ messages in thread
From: Lars-Peter Clausen @ 2012-11-15 13:15 UTC (permalink / raw)
  To: Jonathan Cameron; +Cc: linux-iio, drivers, Lars-Peter Clausen

The driver does not expose any custom API to userspace and none of the standard
static code checker tools report any issues, so move it out of staging.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/iio/adc/Kconfig                                      | 12 ++++++++++++
 drivers/iio/adc/Makefile                                     |  1 +
 drivers/{staging => }/iio/adc/ad7298.c                       |  2 +-
 drivers/staging/iio/adc/Kconfig                              | 12 ------------
 drivers/staging/iio/adc/Makefile                             |  1 -
 .../staging/iio/adc => include/linux/platform_data}/ad7298.h |  4 ++--
 6 files changed, 16 insertions(+), 16 deletions(-)
 rename drivers/{staging => }/iio/adc/ad7298.c (99%)
 rename {drivers/staging/iio/adc => include/linux/platform_data}/ad7298.h (80%)

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index ef5200a..cd5eed6 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -18,6 +18,18 @@ config AD7266
 	  Say yes here to build support for Analog Devices AD7265 and AD7266
 	  ADCs.
 
+config AD7298
+	tristate "Analog Devices AD7298 ADC driver"
+	depends on SPI
+	select IIO_BUFFER
+	select IIO_TRIGGERED_BUFFER
+	help
+	  Say yes here to build support for Analog Devices AD7298
+	  8 Channel ADC with temperature sensor.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called ad7298.
+
 config AD7791
 	tristate "Analog Devices AD7791 ADC driver"
 	depends on SPI
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index 54ac7bb..3256dc6 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -4,6 +4,7 @@
 
 obj-$(CONFIG_AD_SIGMA_DELTA) += ad_sigma_delta.o
 obj-$(CONFIG_AD7266) += ad7266.o
+obj-$(CONFIG_AD7298) += ad7298.o
 obj-$(CONFIG_AD7476) += ad7476.o
 obj-$(CONFIG_AD7791) += ad7791.o
 obj-$(CONFIG_AD7887) += ad7887.o
diff --git a/drivers/staging/iio/adc/ad7298.c b/drivers/iio/adc/ad7298.c
similarity index 99%
rename from drivers/staging/iio/adc/ad7298.c
rename to drivers/iio/adc/ad7298.c
index 2742a9d..441a9a2 100644
--- a/drivers/staging/iio/adc/ad7298.c
+++ b/drivers/iio/adc/ad7298.c
@@ -23,7 +23,7 @@
 #include <linux/iio/trigger_consumer.h>
 #include <linux/iio/triggered_buffer.h>
 
-#include "ad7298.h"
+#include <linux/platform_data/ad7298.h>
 
 #define AD7298_WRITE	(1 << 15) /* write to the control register */
 #define AD7298_REPEAT	(1 << 14) /* repeated conversion enable */
diff --git a/drivers/staging/iio/adc/Kconfig b/drivers/staging/iio/adc/Kconfig
index 5086a46..dc8582b 100644
--- a/drivers/staging/iio/adc/Kconfig
+++ b/drivers/staging/iio/adc/Kconfig
@@ -10,18 +10,6 @@ config AD7291
 	  Say yes here to build support for Analog Devices AD7291
 	  8 Channel ADC with temperature sensor.
 
-config AD7298
-	tristate "Analog Devices AD7298 ADC driver"
-	depends on SPI
-	select IIO_BUFFER
-	select IIO_TRIGGERED_BUFFER
-	help
-	  Say yes here to build support for Analog Devices AD7298
-	  8 Channel ADC with temperature sensor.
-
-	  To compile this driver as a module, choose M here: the
-	  module will be called ad7298.
-
 config AD7606
 	tristate "Analog Devices AD7606 ADC driver"
 	depends on GPIOLIB
diff --git a/drivers/staging/iio/adc/Makefile b/drivers/staging/iio/adc/Makefile
index e867d01..a80e603 100644
--- a/drivers/staging/iio/adc/Makefile
+++ b/drivers/staging/iio/adc/Makefile
@@ -13,7 +13,6 @@ ad799x-$(CONFIG_AD799X_RING_BUFFER) += ad799x_ring.o
 obj-$(CONFIG_AD799X) += ad799x.o
 
 obj-$(CONFIG_AD7887) += ad7887.o
-obj-$(CONFIG_AD7298) += ad7298.o
 
 obj-$(CONFIG_AD7291) += ad7291.o
 obj-$(CONFIG_AD7780) += ad7780.o
diff --git a/drivers/staging/iio/adc/ad7298.h b/include/linux/platform_data/ad7298.h
similarity index 80%
rename from drivers/staging/iio/adc/ad7298.h
rename to include/linux/platform_data/ad7298.h
index c8ac969..fbf8adf 100644
--- a/drivers/staging/iio/adc/ad7298.h
+++ b/include/linux/platform_data/ad7298.h
@@ -6,8 +6,8 @@
  * Licensed under the GPL-2.
  */
 
-#ifndef IIO_ADC_AD7298_H_
-#define IIO_ADC_AD7298_H_
+#ifndef __LINUX_PLATFORM_DATA_AD7298_H__
+#define __LINUX_PLATFORM_DATA_AD7298_H__
 
 /**
  * struct ad7298_platform_data - Platform data for the ad7298 ADC driver
-- 
1.8.0

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/5] staging:iio:ad7298: Do not perform endianness conversion in buffered mode
  2012-11-15 13:15 [PATCH 1/5] staging:iio:ad7298: Do not perform endianness conversion in buffered mode Lars-Peter Clausen
                   ` (3 preceding siblings ...)
  2012-11-15 13:15 ` [PATCH 5/5] staging:iio: Move the ad7298 driver out of staging Lars-Peter Clausen
@ 2012-11-17 11:16 ` Jonathan Cameron
  4 siblings, 0 replies; 12+ messages in thread
From: Jonathan Cameron @ 2012-11-17 11:16 UTC (permalink / raw)
  To: Lars-Peter Clausen; +Cc: Jonathan Cameron, linux-iio, drivers

On 11/15/2012 01:15 PM, Lars-Peter Clausen wrote:
> For buffered mode we do not want to perform endianness conversion in the kernel,
> but rather offload it to user space, since it is not always required to do a
> conversion at all. It also greatly simplifies the kernel code since no
> post-processing has to be done and may allow future optimizations like streaming
> data directly to a storage device or over the network via DMA.
>
added to togreg branch of iio.git
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> ---
>  drivers/staging/iio/adc/ad7298.h      |  2 +-
>  drivers/staging/iio/adc/ad7298_core.c |  1 +
>  drivers/staging/iio/adc/ad7298_ring.c | 11 +++--------
>  3 files changed, 5 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/staging/iio/adc/ad7298.h b/drivers/staging/iio/adc/ad7298.h
> index 18f2787..0ce9031 100644
> --- a/drivers/staging/iio/adc/ad7298.h
> +++ b/drivers/staging/iio/adc/ad7298.h
> @@ -48,7 +48,7 @@ struct ad7298_state {
>  	 * DMA (thus cache coherency maintenance) requires the
>  	 * transfer buffers to live in their own cache lines.
>  	 */
> -	unsigned short			rx_buf[8] ____cacheline_aligned;
> +	unsigned short			rx_buf[12] ____cacheline_aligned;
>  	unsigned short			tx_buf[2];
>  };
>  
> diff --git a/drivers/staging/iio/adc/ad7298_core.c b/drivers/staging/iio/adc/ad7298_core.c
> index 4c75114..67082ad 100644
> --- a/drivers/staging/iio/adc/ad7298_core.c
> +++ b/drivers/staging/iio/adc/ad7298_core.c
> @@ -35,6 +35,7 @@
>  			.sign = 'u',					\
>  			.realbits = 12,					\
>  			.storagebits = 16,				\
> +			.endianness = IIO_BE,				\
>  		},							\
>  	}
>  
> diff --git a/drivers/staging/iio/adc/ad7298_ring.c b/drivers/staging/iio/adc/ad7298_ring.c
> index b3dd514..e387712 100644
> --- a/drivers/staging/iio/adc/ad7298_ring.c
> +++ b/drivers/staging/iio/adc/ad7298_ring.c
> @@ -76,8 +76,7 @@ static irqreturn_t ad7298_trigger_handler(int irq, void *p)
>  	struct iio_dev *indio_dev = pf->indio_dev;
>  	struct ad7298_state *st = iio_priv(indio_dev);
>  	s64 time_ns = 0;
> -	__u16 buf[16];
> -	int b_sent, i;
> +	int b_sent;
>  
>  	b_sent = spi_sync(st->spi, &st->ring_msg);
>  	if (b_sent)
> @@ -85,15 +84,11 @@ static irqreturn_t ad7298_trigger_handler(int irq, void *p)
>  
>  	if (indio_dev->scan_timestamp) {
>  		time_ns = iio_get_time_ns();
> -		memcpy((u8 *)buf + indio_dev->scan_bytes - sizeof(s64),
> +		memcpy((u8 *)st->rx_buf + indio_dev->scan_bytes - sizeof(s64),
>  			&time_ns, sizeof(time_ns));
>  	}
>  
> -	for (i = 0; i < bitmap_weight(indio_dev->active_scan_mask,
> -						 indio_dev->masklength); i++)
> -		buf[i] = be16_to_cpu(st->rx_buf[i]);
> -
> -	iio_push_to_buffers(indio_dev, (u8 *)buf);
> +	iio_push_to_buffers(indio_dev, (u8 *)st->rx_buf);
>  
>  done:
>  	iio_trigger_notify_done(indio_dev->trig);
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/5] staging:iio:ad7298: Rework regulator handling
  2012-11-15 13:15 ` [PATCH 2/5] staging:iio:ad7298: Rework regulator handling Lars-Peter Clausen
@ 2012-11-17 11:17   ` Jonathan Cameron
  0 siblings, 0 replies; 12+ messages in thread
From: Jonathan Cameron @ 2012-11-17 11:17 UTC (permalink / raw)
  To: Lars-Peter Clausen; +Cc: Jonathan Cameron, linux-iio, drivers

On 11/15/2012 01:15 PM, Lars-Peter Clausen wrote:
> Rework the regulator handling of the driver to match more closely what we do in
> other drivers. Make the regulator non-optional if a external reference is used.
> Also dispose the option of specifying the reference voltage via platform data.
Added to togreg branch of iio.git.
> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> ---
>  drivers/staging/iio/adc/ad7298.h      | 12 ++++------
>  drivers/staging/iio/adc/ad7298_core.c | 44 +++++++++++++++++++++--------------
>  2 files changed, 31 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/staging/iio/adc/ad7298.h b/drivers/staging/iio/adc/ad7298.h
> index 0ce9031..6523e01 100644
> --- a/drivers/staging/iio/adc/ad7298.h
> +++ b/drivers/staging/iio/adc/ad7298.h
> @@ -26,19 +26,17 @@
>  
>  #define RES_MASK(bits)	((1 << (bits)) - 1)
>  
> -/*
> - * TODO: struct ad7298_platform_data needs to go into include/linux/iio
> - */
> -
> +/**
> + * struct ad7298_platform_data - Platform data for the ad7298 ADC driver
> + * @ext_ref: Whether to use an external reference voltage.
> + **/
>  struct ad7298_platform_data {
> -	/* External Vref voltage applied */
> -	u16				vref_mv;
> +	bool ext_ref;
>  };
>  
>  struct ad7298_state {
>  	struct spi_device		*spi;
>  	struct regulator		*reg;
> -	u16				int_vref_mv;
>  	unsigned			ext_ref;
>  	struct spi_transfer		ring_xfer[10];
>  	struct spi_transfer		scan_single_xfer[3];
> diff --git a/drivers/staging/iio/adc/ad7298_core.c b/drivers/staging/iio/adc/ad7298_core.c
> index 67082ad..6dd696f 100644
> --- a/drivers/staging/iio/adc/ad7298_core.c
> +++ b/drivers/staging/iio/adc/ad7298_core.c
> @@ -130,7 +130,7 @@ static int ad7298_read_raw(struct iio_dev *indio_dev,
>  {
>  	int ret;
>  	struct ad7298_state *st = iio_priv(indio_dev);
> -	unsigned int scale_uv;
> +	int scale_mv;
>  
>  	switch (m) {
>  	case IIO_CHAN_INFO_RAW:
> @@ -155,10 +155,17 @@ static int ad7298_read_raw(struct iio_dev *indio_dev,
>  	case IIO_CHAN_INFO_SCALE:
>  		switch (chan->type) {
>  		case IIO_VOLTAGE:
> -			scale_uv = (st->int_vref_mv * 1000) >> AD7298_BITS;
> -			*val =  scale_uv / 1000;
> -			*val2 = (scale_uv % 1000) * 1000;
> -			return IIO_VAL_INT_PLUS_MICRO;
> +			if (st->ext_ref) {
> +				scale_mv = regulator_get_voltage(st->reg);
> +				if (scale_mv < 0)
> +					return scale_mv;
> +				scale_mv /= 1000;
> +			} else {
> +				scale_mv = AD7298_INTREF_mV;
> +			}
> +			*val =  scale_mv;
> +			*val2 = chan->scan_type.realbits;
> +			return IIO_VAL_FRACTIONAL_LOG2;
>  		case IIO_TEMP:
>  			*val =  1;
>  			*val2 = 0;
> @@ -180,16 +187,23 @@ static int __devinit ad7298_probe(struct spi_device *spi)
>  {
>  	struct ad7298_platform_data *pdata = spi->dev.platform_data;
>  	struct ad7298_state *st;
> -	int ret;
>  	struct iio_dev *indio_dev = iio_device_alloc(sizeof(*st));
> +	int ret;
>  
>  	if (indio_dev == NULL)
>  		return -ENOMEM;
>  
>  	st = iio_priv(indio_dev);
>  
> -	st->reg = regulator_get(&spi->dev, "vcc");
> -	if (!IS_ERR(st->reg)) {
> +	if (pdata && pdata->ext_ref)
> +		st->ext_ref = AD7298_EXTREF;
> +
> +	if (st->ext_ref) {
> +		st->reg = regulator_get(&spi->dev, "vref");
> +		if (IS_ERR(st->reg)) {
> +			ret = PTR_ERR(st->reg);
> +			goto error_free;
> +		}
>  		ret = regulator_enable(st->reg);
>  		if (ret)
>  			goto error_put_reg;
> @@ -222,13 +236,6 @@ static int __devinit ad7298_probe(struct spi_device *spi)
>  	spi_message_add_tail(&st->scan_single_xfer[1], &st->scan_single_msg);
>  	spi_message_add_tail(&st->scan_single_xfer[2], &st->scan_single_msg);
>  
> -	if (pdata && pdata->vref_mv) {
> -		st->int_vref_mv = pdata->vref_mv;
> -		st->ext_ref = AD7298_EXTREF;
> -	} else {
> -		st->int_vref_mv = AD7298_INTREF_mV;
> -	}
> -
>  	ret = ad7298_register_ring_funcs_and_init(indio_dev);
>  	if (ret)
>  		goto error_disable_reg;
> @@ -242,11 +249,12 @@ static int __devinit ad7298_probe(struct spi_device *spi)
>  error_cleanup_ring:
>  	ad7298_ring_cleanup(indio_dev);
>  error_disable_reg:
> -	if (!IS_ERR(st->reg))
> +	if (st->ext_ref)
>  		regulator_disable(st->reg);
>  error_put_reg:
> -	if (!IS_ERR(st->reg))
> +	if (st->ext_ref)
>  		regulator_put(st->reg);
> +error_free:
>  	iio_device_free(indio_dev);
>  
>  	return ret;
> @@ -259,7 +267,7 @@ static int __devexit ad7298_remove(struct spi_device *spi)
>  
>  	iio_device_unregister(indio_dev);
>  	ad7298_ring_cleanup(indio_dev);
> -	if (!IS_ERR(st->reg)) {
> +	if (st->ext_ref) {
>  		regulator_disable(st->reg);
>  		regulator_put(st->reg);
>  	}
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/5] staging:iio:ad7298: Fix temperature scale and offset
  2012-11-15 13:15 ` [PATCH 3/5] staging:iio:ad7298: Fix temperature scale and offset Lars-Peter Clausen
@ 2012-11-17 11:19   ` Jonathan Cameron
  0 siblings, 0 replies; 12+ messages in thread
From: Jonathan Cameron @ 2012-11-17 11:19 UTC (permalink / raw)
  To: Lars-Peter Clausen; +Cc: Jonathan Cameron, linux-iio, drivers

On 11/15/2012 01:15 PM, Lars-Peter Clausen wrote:
> The temperature scale and offset depend on the reference voltage, the current
> formula used in the driver assumes a 2.5V reference. This patch modifies the
> code to report the unprocessed value for the temperature channel "raw" property
> and to provide proper "scale" and "offset" properties which depend on the
> selected reference voltage.
added to togreg branch of iio.git. Thanks.

> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> ---
>  drivers/staging/iio/adc/ad7298_core.c | 51 ++++++++++++++++-------------------
>  1 file changed, 23 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/staging/iio/adc/ad7298_core.c b/drivers/staging/iio/adc/ad7298_core.c
> index 6dd696f..b74b76f 100644
> --- a/drivers/staging/iio/adc/ad7298_core.c
> +++ b/drivers/staging/iio/adc/ad7298_core.c
> @@ -45,7 +45,8 @@ static const struct iio_chan_spec ad7298_channels[] = {
>  		.indexed = 1,
>  		.channel = 0,
>  		.info_mask = IIO_CHAN_INFO_RAW_SEPARATE_BIT |
> -		IIO_CHAN_INFO_SCALE_SEPARATE_BIT,
> +			IIO_CHAN_INFO_SCALE_SEPARATE_BIT |
> +			IIO_CHAN_INFO_OFFSET_SEPARATE_BIT,
>  		.address = AD7298_CH_TEMP,
>  		.scan_index = -1,
>  		.scan_type = {
> @@ -80,7 +81,7 @@ static int ad7298_scan_direct(struct ad7298_state *st, unsigned ch)
>  
>  static int ad7298_scan_temp(struct ad7298_state *st, int *val)
>  {
> -	int tmp, ret;
> +	int ret;
>  	__be16 buf;
>  
>  	buf = cpu_to_be16(AD7298_WRITE | AD7298_TSENSE |
> @@ -102,24 +103,24 @@ static int ad7298_scan_temp(struct ad7298_state *st, int *val)
>  	if (ret)
>  		return ret;
>  
> -	tmp = be16_to_cpu(buf) & RES_MASK(AD7298_BITS);
> +	*val = sign_extend32(be16_to_cpu(buf), 11);
>  
> -	/*
> -	 * One LSB of the ADC corresponds to 0.25 deg C.
> -	 * The temperature reading is in 12-bit twos complement format
> -	 */
> +	return 0;
> +}
> +
> +static int ad7298_get_ref_voltage(struct ad7298_state *st)
> +{
> +	int vref;
>  
> -	if (tmp & (1 << (AD7298_BITS - 1))) {
> -		tmp = (4096 - tmp) * 250;
> -		tmp -= (2 * tmp);
> +	if (st->ext_ref) {
> +		vref = regulator_get_voltage(st->reg);
> +		if (vref < 0)
> +			return vref;
>  
> +		return vref / 1000;
>  	} else {
> -		tmp *= 250; /* temperature in milli degrees Celsius */
> +		return AD7298_INTREF_mV;
>  	}
> -
> -	*val = tmp;
> -
> -	return 0;
>  }
>  
>  static int ad7298_read_raw(struct iio_dev *indio_dev,
> @@ -130,7 +131,6 @@ static int ad7298_read_raw(struct iio_dev *indio_dev,
>  {
>  	int ret;
>  	struct ad7298_state *st = iio_priv(indio_dev);
> -	int scale_mv;
>  
>  	switch (m) {
>  	case IIO_CHAN_INFO_RAW:
> @@ -155,24 +155,19 @@ static int ad7298_read_raw(struct iio_dev *indio_dev,
>  	case IIO_CHAN_INFO_SCALE:
>  		switch (chan->type) {
>  		case IIO_VOLTAGE:
> -			if (st->ext_ref) {
> -				scale_mv = regulator_get_voltage(st->reg);
> -				if (scale_mv < 0)
> -					return scale_mv;
> -				scale_mv /= 1000;
> -			} else {
> -				scale_mv = AD7298_INTREF_mV;
> -			}
> -			*val =  scale_mv;
> +			*val = ad7298_get_ref_voltage(st);
>  			*val2 = chan->scan_type.realbits;
>  			return IIO_VAL_FRACTIONAL_LOG2;
>  		case IIO_TEMP:
> -			*val =  1;
> -			*val2 = 0;
> -			return IIO_VAL_INT_PLUS_MICRO;
> +			*val = ad7298_get_ref_voltage(st);
> +			*val2 = 10;
> +			return IIO_VAL_FRACTIONAL;
>  		default:
>  			return -EINVAL;
>  		}
> +	case IIO_CHAN_INFO_OFFSET:
> +		*val = 1093 - 2732500 / ad7298_get_ref_voltage(st);
> +		return IIO_VAL_INT;
>  	}
>  	return -EINVAL;
>  }
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 4/5] staging:iio:ad7298: Squash everything into one file
  2012-11-15 13:15 ` [PATCH 4/5] staging:iio:ad7298: Squash everything into one file Lars-Peter Clausen
@ 2012-11-17 11:20   ` Jonathan Cameron
  2012-11-17 11:34     ` Jonathan Cameron
  0 siblings, 1 reply; 12+ messages in thread
From: Jonathan Cameron @ 2012-11-17 11:20 UTC (permalink / raw)
  To: Lars-Peter Clausen; +Cc: Jonathan Cameron, linux-iio, drivers

On 11/15/2012 01:15 PM, Lars-Peter Clausen wrote:
> The recent cleanups have decimated the drivers code size by quite a bit. It is
> only a few hundred lines in total now. Putting everything into one file also
> allows to reduce the code size a bit more by removing a few lines of boilerplate
> code. The only functional change made by this patch is that we now always
> include buffer support, instead of making it optional. This is more consistent
> with what we do for other drivers.
> 
added to togreg branch of iio.git
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> ---
>  drivers/staging/iio/adc/Kconfig                    |   3 +-
>  drivers/staging/iio/adc/Makefile                   |   3 +-
>  .../staging/iio/adc/{ad7298_core.c => ad7298.c}    | 121 ++++++++++++++++++++-
>  drivers/staging/iio/adc/ad7298.h                   |  53 ---------
>  drivers/staging/iio/adc/ad7298_ring.c              | 108 ------------------
>  5 files changed, 121 insertions(+), 167 deletions(-)
>  rename drivers/staging/iio/adc/{ad7298_core.c => ad7298.c} (64%)
>  delete mode 100644 drivers/staging/iio/adc/ad7298_ring.c
> 
> diff --git a/drivers/staging/iio/adc/Kconfig b/drivers/staging/iio/adc/Kconfig
> index 0177f1e..5086a46 100644
> --- a/drivers/staging/iio/adc/Kconfig
> +++ b/drivers/staging/iio/adc/Kconfig
> @@ -13,7 +13,8 @@ config AD7291
>  config AD7298
>  	tristate "Analog Devices AD7298 ADC driver"
>  	depends on SPI
> -	select IIO_TRIGGERED_BUFFER if IIO_BUFFER
> +	select IIO_BUFFER
> +	select IIO_TRIGGERED_BUFFER
>  	help
>  	  Say yes here to build support for Analog Devices AD7298
>  	  8 Channel ADC with temperature sensor.
> diff --git a/drivers/staging/iio/adc/Makefile b/drivers/staging/iio/adc/Makefile
> index 12b4bd3..e867d01 100644
> --- a/drivers/staging/iio/adc/Makefile
> +++ b/drivers/staging/iio/adc/Makefile
> @@ -12,8 +12,7 @@ ad799x-y := ad799x_core.o
>  ad799x-$(CONFIG_AD799X_RING_BUFFER) += ad799x_ring.o
>  obj-$(CONFIG_AD799X) += ad799x.o
>  
> -ad7298-y := ad7298_core.o
> -ad7298-$(CONFIG_IIO_BUFFER) += ad7298_ring.o
> +obj-$(CONFIG_AD7887) += ad7887.o
>  obj-$(CONFIG_AD7298) += ad7298.o
>  
>  obj-$(CONFIG_AD7291) += ad7291.o
> diff --git a/drivers/staging/iio/adc/ad7298_core.c b/drivers/staging/iio/adc/ad7298.c
> similarity index 64%
> rename from drivers/staging/iio/adc/ad7298_core.c
> rename to drivers/staging/iio/adc/ad7298.c
> index b74b76f..2742a9d 100644
> --- a/drivers/staging/iio/adc/ad7298_core.c
> +++ b/drivers/staging/iio/adc/ad7298.c
> @@ -15,13 +15,49 @@
>  #include <linux/err.h>
>  #include <linux/delay.h>
>  #include <linux/module.h>
> +#include <linux/interrupt.h>
>  
>  #include <linux/iio/iio.h>
>  #include <linux/iio/sysfs.h>
>  #include <linux/iio/buffer.h>
> +#include <linux/iio/trigger_consumer.h>
> +#include <linux/iio/triggered_buffer.h>
>  
>  #include "ad7298.h"
>  
> +#define AD7298_WRITE	(1 << 15) /* write to the control register */
> +#define AD7298_REPEAT	(1 << 14) /* repeated conversion enable */
> +#define AD7298_CH(x)	(1 << (13 - (x))) /* channel select */
> +#define AD7298_TSENSE	(1 << 5) /* temperature conversion enable */
> +#define AD7298_EXTREF	(1 << 2) /* external reference enable */
> +#define AD7298_TAVG	(1 << 1) /* temperature sensor averaging enable */
> +#define AD7298_PDD	(1 << 0) /* partial power down enable */
> +
> +#define AD7298_MAX_CHAN		8
> +#define AD7298_BITS		12
> +#define AD7298_STORAGE_BITS	16
> +#define AD7298_INTREF_mV	2500
> +
> +#define AD7298_CH_TEMP		9
> +
> +#define RES_MASK(bits)	((1 << (bits)) - 1)
> +
> +struct ad7298_state {
> +	struct spi_device		*spi;
> +	struct regulator		*reg;
> +	unsigned			ext_ref;
> +	struct spi_transfer		ring_xfer[10];
> +	struct spi_transfer		scan_single_xfer[3];
> +	struct spi_message		ring_msg;
> +	struct spi_message		scan_single_msg;
> +	/*
> +	 * DMA (thus cache coherency maintenance) requires the
> +	 * transfer buffers to live in their own cache lines.
> +	 */
> +	unsigned short			rx_buf[12] ____cacheline_aligned;
> +	unsigned short			tx_buf[2];
> +};
> +
>  #define AD7298_V_CHAN(index)						\
>  	{								\
>  		.type = IIO_VOLTAGE,					\
> @@ -66,6 +102,84 @@ static const struct iio_chan_spec ad7298_channels[] = {
>  	IIO_CHAN_SOFT_TIMESTAMP(8),
>  };
>  
> +/**
> + * ad7298_update_scan_mode() setup the spi transfer buffer for the new scan mask
> + **/
> +static int ad7298_update_scan_mode(struct iio_dev *indio_dev,
> +	const unsigned long *active_scan_mask)
> +{
> +	struct ad7298_state *st = iio_priv(indio_dev);
> +	int i, m;
> +	unsigned short command;
> +	int scan_count;
> +
> +	/* Now compute overall size */
> +	scan_count = bitmap_weight(active_scan_mask, indio_dev->masklength);
> +
> +	command = AD7298_WRITE | st->ext_ref;
> +
> +	for (i = 0, m = AD7298_CH(0); i < AD7298_MAX_CHAN; i++, m >>= 1)
> +		if (test_bit(i, active_scan_mask))
> +			command |= m;
> +
> +	st->tx_buf[0] = cpu_to_be16(command);
> +
> +	/* build spi ring message */
> +	st->ring_xfer[0].tx_buf = &st->tx_buf[0];
> +	st->ring_xfer[0].len = 2;
> +	st->ring_xfer[0].cs_change = 1;
> +	st->ring_xfer[1].tx_buf = &st->tx_buf[1];
> +	st->ring_xfer[1].len = 2;
> +	st->ring_xfer[1].cs_change = 1;
> +
> +	spi_message_init(&st->ring_msg);
> +	spi_message_add_tail(&st->ring_xfer[0], &st->ring_msg);
> +	spi_message_add_tail(&st->ring_xfer[1], &st->ring_msg);
> +
> +	for (i = 0; i < scan_count; i++) {
> +		st->ring_xfer[i + 2].rx_buf = &st->rx_buf[i];
> +		st->ring_xfer[i + 2].len = 2;
> +		st->ring_xfer[i + 2].cs_change = 1;
> +		spi_message_add_tail(&st->ring_xfer[i + 2], &st->ring_msg);
> +	}
> +	/* make sure last transfer cs_change is not set */
> +	st->ring_xfer[i + 1].cs_change = 0;
> +
> +	return 0;
> +}
> +
> +/**
> + * ad7298_trigger_handler() bh of trigger launched polling to ring buffer
> + *
> + * Currently there is no option in this driver to disable the saving of
> + * timestamps within the ring.
> + **/
> +static irqreturn_t ad7298_trigger_handler(int irq, void *p)
> +{
> +	struct iio_poll_func *pf = p;
> +	struct iio_dev *indio_dev = pf->indio_dev;
> +	struct ad7298_state *st = iio_priv(indio_dev);
> +	s64 time_ns = 0;
> +	int b_sent;
> +
> +	b_sent = spi_sync(st->spi, &st->ring_msg);
> +	if (b_sent)
> +		goto done;
> +
> +	if (indio_dev->scan_timestamp) {
> +		time_ns = iio_get_time_ns();
> +		memcpy((u8 *)st->rx_buf + indio_dev->scan_bytes - sizeof(s64),
> +			&time_ns, sizeof(time_ns));
> +	}
> +
> +	iio_push_to_buffers(indio_dev, (u8 *)st->rx_buf);
> +
> +done:
> +	iio_trigger_notify_done(indio_dev->trig);
> +
> +	return IRQ_HANDLED;
> +}
> +
>  static int ad7298_scan_direct(struct ad7298_state *st, unsigned ch)
>  {
>  	int ret;
> @@ -231,7 +345,8 @@ static int __devinit ad7298_probe(struct spi_device *spi)
>  	spi_message_add_tail(&st->scan_single_xfer[1], &st->scan_single_msg);
>  	spi_message_add_tail(&st->scan_single_xfer[2], &st->scan_single_msg);
>  
> -	ret = ad7298_register_ring_funcs_and_init(indio_dev);
> +	ret = iio_triggered_buffer_setup(indio_dev, NULL,
> +			&ad7298_trigger_handler, NULL);
>  	if (ret)
>  		goto error_disable_reg;
>  
> @@ -242,7 +357,7 @@ static int __devinit ad7298_probe(struct spi_device *spi)
>  	return 0;
>  
>  error_cleanup_ring:
> -	ad7298_ring_cleanup(indio_dev);
> +	iio_triggered_buffer_cleanup(indio_dev);
>  error_disable_reg:
>  	if (st->ext_ref)
>  		regulator_disable(st->reg);
> @@ -261,7 +376,7 @@ static int __devexit ad7298_remove(struct spi_device *spi)
>  	struct ad7298_state *st = iio_priv(indio_dev);
>  
>  	iio_device_unregister(indio_dev);
> -	ad7298_ring_cleanup(indio_dev);
> +	iio_triggered_buffer_cleanup(indio_dev);
>  	if (st->ext_ref) {
>  		regulator_disable(st->reg);
>  		regulator_put(st->reg);
> diff --git a/drivers/staging/iio/adc/ad7298.h b/drivers/staging/iio/adc/ad7298.h
> index 6523e01..c8ac969 100644
> --- a/drivers/staging/iio/adc/ad7298.h
> +++ b/drivers/staging/iio/adc/ad7298.h
> @@ -9,23 +9,6 @@
>  #ifndef IIO_ADC_AD7298_H_
>  #define IIO_ADC_AD7298_H_
>  
> -#define AD7298_WRITE	(1 << 15) /* write to the control register */
> -#define AD7298_REPEAT	(1 << 14) /* repeated conversion enable */
> -#define AD7298_CH(x)	(1 << (13 - (x))) /* channel select */
> -#define AD7298_TSENSE	(1 << 5) /* temperature conversion enable */
> -#define AD7298_EXTREF	(1 << 2) /* external reference enable */
> -#define AD7298_TAVG	(1 << 1) /* temperature sensor averaging enable */
> -#define AD7298_PDD	(1 << 0) /* partial power down enable */
> -
> -#define AD7298_MAX_CHAN		8
> -#define AD7298_BITS		12
> -#define AD7298_STORAGE_BITS	16
> -#define AD7298_INTREF_mV	2500
> -
> -#define AD7298_CH_TEMP		9
> -
> -#define RES_MASK(bits)	((1 << (bits)) - 1)
> -
>  /**
>   * struct ad7298_platform_data - Platform data for the ad7298 ADC driver
>   * @ext_ref: Whether to use an external reference voltage.
> @@ -34,40 +17,4 @@ struct ad7298_platform_data {
>  	bool ext_ref;
>  };
>  
> -struct ad7298_state {
> -	struct spi_device		*spi;
> -	struct regulator		*reg;
> -	unsigned			ext_ref;
> -	struct spi_transfer		ring_xfer[10];
> -	struct spi_transfer		scan_single_xfer[3];
> -	struct spi_message		ring_msg;
> -	struct spi_message		scan_single_msg;
> -	/*
> -	 * DMA (thus cache coherency maintenance) requires the
> -	 * transfer buffers to live in their own cache lines.
> -	 */
> -	unsigned short			rx_buf[12] ____cacheline_aligned;
> -	unsigned short			tx_buf[2];
> -};
> -
> -#ifdef CONFIG_IIO_BUFFER
> -int ad7298_register_ring_funcs_and_init(struct iio_dev *indio_dev);
> -void ad7298_ring_cleanup(struct iio_dev *indio_dev);
> -int ad7298_update_scan_mode(struct iio_dev *indio_dev,
> -	const unsigned long *active_scan_mask);
> -#else /* CONFIG_IIO_BUFFER */
> -
> -static inline int
> -ad7298_register_ring_funcs_and_init(struct iio_dev *indio_dev)
> -{
> -	return 0;
> -}
> -
> -static inline void ad7298_ring_cleanup(struct iio_dev *indio_dev)
> -{
> -}
> -
> -#define ad7298_update_scan_mode NULL
> -
> -#endif /* CONFIG_IIO_BUFFER */
>  #endif /* IIO_ADC_AD7298_H_ */
> diff --git a/drivers/staging/iio/adc/ad7298_ring.c b/drivers/staging/iio/adc/ad7298_ring.c
> deleted file mode 100644
> index e387712..0000000
> --- a/drivers/staging/iio/adc/ad7298_ring.c
> +++ /dev/null
> @@ -1,108 +0,0 @@
> -/*
> - * AD7298 SPI ADC driver
> - *
> - * Copyright 2011-2012 Analog Devices Inc.
> - *
> - * Licensed under the GPL-2.
> - */
> -
> -#include <linux/interrupt.h>
> -#include <linux/kernel.h>
> -#include <linux/slab.h>
> -#include <linux/spi/spi.h>
> -
> -#include <linux/iio/iio.h>
> -#include <linux/iio/buffer.h>
> -#include <linux/iio/trigger_consumer.h>
> -#include <linux/iio/triggered_buffer.h>
> -
> -#include "ad7298.h"
> -
> -/**
> - * ad7298_update_scan_mode() setup the spi transfer buffer for the new scan mask
> - **/
> -int ad7298_update_scan_mode(struct iio_dev *indio_dev,
> -	const unsigned long *active_scan_mask)
> -{
> -	struct ad7298_state *st = iio_priv(indio_dev);
> -	int i, m;
> -	unsigned short command;
> -	int scan_count;
> -
> -	/* Now compute overall size */
> -	scan_count = bitmap_weight(active_scan_mask, indio_dev->masklength);
> -
> -	command = AD7298_WRITE | st->ext_ref;
> -
> -	for (i = 0, m = AD7298_CH(0); i < AD7298_MAX_CHAN; i++, m >>= 1)
> -		if (test_bit(i, active_scan_mask))
> -			command |= m;
> -
> -	st->tx_buf[0] = cpu_to_be16(command);
> -
> -	/* build spi ring message */
> -	st->ring_xfer[0].tx_buf = &st->tx_buf[0];
> -	st->ring_xfer[0].len = 2;
> -	st->ring_xfer[0].cs_change = 1;
> -	st->ring_xfer[1].tx_buf = &st->tx_buf[1];
> -	st->ring_xfer[1].len = 2;
> -	st->ring_xfer[1].cs_change = 1;
> -
> -	spi_message_init(&st->ring_msg);
> -	spi_message_add_tail(&st->ring_xfer[0], &st->ring_msg);
> -	spi_message_add_tail(&st->ring_xfer[1], &st->ring_msg);
> -
> -	for (i = 0; i < scan_count; i++) {
> -		st->ring_xfer[i + 2].rx_buf = &st->rx_buf[i];
> -		st->ring_xfer[i + 2].len = 2;
> -		st->ring_xfer[i + 2].cs_change = 1;
> -		spi_message_add_tail(&st->ring_xfer[i + 2], &st->ring_msg);
> -	}
> -	/* make sure last transfer cs_change is not set */
> -	st->ring_xfer[i + 1].cs_change = 0;
> -
> -	return 0;
> -}
> -
> -/**
> - * ad7298_trigger_handler() bh of trigger launched polling to ring buffer
> - *
> - * Currently there is no option in this driver to disable the saving of
> - * timestamps within the ring.
> - **/
> -static irqreturn_t ad7298_trigger_handler(int irq, void *p)
> -{
> -	struct iio_poll_func *pf = p;
> -	struct iio_dev *indio_dev = pf->indio_dev;
> -	struct ad7298_state *st = iio_priv(indio_dev);
> -	s64 time_ns = 0;
> -	int b_sent;
> -
> -	b_sent = spi_sync(st->spi, &st->ring_msg);
> -	if (b_sent)
> -		goto done;
> -
> -	if (indio_dev->scan_timestamp) {
> -		time_ns = iio_get_time_ns();
> -		memcpy((u8 *)st->rx_buf + indio_dev->scan_bytes - sizeof(s64),
> -			&time_ns, sizeof(time_ns));
> -	}
> -
> -	iio_push_to_buffers(indio_dev, (u8 *)st->rx_buf);
> -
> -done:
> -	iio_trigger_notify_done(indio_dev->trig);
> -
> -	return IRQ_HANDLED;
> -}
> -
> -int ad7298_register_ring_funcs_and_init(struct iio_dev *indio_dev)
> -{
> -	return iio_triggered_buffer_setup(indio_dev, NULL,
> -			&ad7298_trigger_handler, NULL);
> -}
> -
> -void ad7298_ring_cleanup(struct iio_dev *indio_dev)
> -{
> -	iio_triggered_buffer_cleanup(indio_dev);
> -}
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 4/5] staging:iio:ad7298: Squash everything into one file
  2012-11-17 11:20   ` Jonathan Cameron
@ 2012-11-17 11:34     ` Jonathan Cameron
  2012-11-17 17:23       ` Lars-Peter Clausen
  0 siblings, 1 reply; 12+ messages in thread
From: Jonathan Cameron @ 2012-11-17 11:34 UTC (permalink / raw)
  To: Lars-Peter Clausen; +Cc: Jonathan Cameron, linux-iio, drivers

On 11/17/2012 11:20 AM, Jonathan Cameron wrote:
> On 11/15/2012 01:15 PM, Lars-Peter Clausen wrote:
>> The recent cleanups have decimated the drivers code size by quite a bit. It is
>> only a few hundred lines in total now. Putting everything into one file also
>> allows to reduce the code size a bit more by removing a few lines of boilerplate
>> code. The only functional change made by this patch is that we now always
>> include buffer support, instead of making it optional. This is more consistent
>> with what we do for other drivers.
>>
> added to togreg branch of iio.git
Actually scratch that - my build test hadn't finished and I jumped the gun.

You have a reference to ad7887 bleeding in from somewhere in the makefile.
I've fixed it up before pushing out


>> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
>> ---
>>  drivers/staging/iio/adc/Kconfig                    |   3 +-
>>  drivers/staging/iio/adc/Makefile                   |   3 +-
>>  .../staging/iio/adc/{ad7298_core.c => ad7298.c}    | 121 ++++++++++++++++++++-
>>  drivers/staging/iio/adc/ad7298.h                   |  53 ---------
>>  drivers/staging/iio/adc/ad7298_ring.c              | 108 ------------------
>>  5 files changed, 121 insertions(+), 167 deletions(-)
>>  rename drivers/staging/iio/adc/{ad7298_core.c => ad7298.c} (64%)
>>  delete mode 100644 drivers/staging/iio/adc/ad7298_ring.c
>>
>> diff --git a/drivers/staging/iio/adc/Kconfig b/drivers/staging/iio/adc/Kconfig
>> index 0177f1e..5086a46 100644
>> --- a/drivers/staging/iio/adc/Kconfig
>> +++ b/drivers/staging/iio/adc/Kconfig
>> @@ -13,7 +13,8 @@ config AD7291
>>  config AD7298
>>  	tristate "Analog Devices AD7298 ADC driver"
>>  	depends on SPI
>> -	select IIO_TRIGGERED_BUFFER if IIO_BUFFER
>> +	select IIO_BUFFER
>> +	select IIO_TRIGGERED_BUFFER
>>  	help
>>  	  Say yes here to build support for Analog Devices AD7298
>>  	  8 Channel ADC with temperature sensor.
>> diff --git a/drivers/staging/iio/adc/Makefile b/drivers/staging/iio/adc/Makefile
>> index 12b4bd3..e867d01 100644
>> --- a/drivers/staging/iio/adc/Makefile
>> +++ b/drivers/staging/iio/adc/Makefile
>> @@ -12,8 +12,7 @@ ad799x-y := ad799x_core.o
>>  ad799x-$(CONFIG_AD799X_RING_BUFFER) += ad799x_ring.o
>>  obj-$(CONFIG_AD799X) += ad799x.o
>>  
>> -ad7298-y := ad7298_core.o
>> -ad7298-$(CONFIG_IIO_BUFFER) += ad7298_ring.o
>> +obj-$(CONFIG_AD7887) += ad7887.o
>>  obj-$(CONFIG_AD7298) += ad7298.o
>>  
>>  obj-$(CONFIG_AD7291) += ad7291.o
>> diff --git a/drivers/staging/iio/adc/ad7298_core.c b/drivers/staging/iio/adc/ad7298.c
>> similarity index 64%
>> rename from drivers/staging/iio/adc/ad7298_core.c
>> rename to drivers/staging/iio/adc/ad7298.c
>> index b74b76f..2742a9d 100644
>> --- a/drivers/staging/iio/adc/ad7298_core.c
>> +++ b/drivers/staging/iio/adc/ad7298.c
>> @@ -15,13 +15,49 @@
>>  #include <linux/err.h>
>>  #include <linux/delay.h>
>>  #include <linux/module.h>
>> +#include <linux/interrupt.h>
>>  
>>  #include <linux/iio/iio.h>
>>  #include <linux/iio/sysfs.h>
>>  #include <linux/iio/buffer.h>
>> +#include <linux/iio/trigger_consumer.h>
>> +#include <linux/iio/triggered_buffer.h>
>>  
>>  #include "ad7298.h"
>>  
>> +#define AD7298_WRITE	(1 << 15) /* write to the control register */
>> +#define AD7298_REPEAT	(1 << 14) /* repeated conversion enable */
>> +#define AD7298_CH(x)	(1 << (13 - (x))) /* channel select */
>> +#define AD7298_TSENSE	(1 << 5) /* temperature conversion enable */
>> +#define AD7298_EXTREF	(1 << 2) /* external reference enable */
>> +#define AD7298_TAVG	(1 << 1) /* temperature sensor averaging enable */
>> +#define AD7298_PDD	(1 << 0) /* partial power down enable */
>> +
>> +#define AD7298_MAX_CHAN		8
>> +#define AD7298_BITS		12
>> +#define AD7298_STORAGE_BITS	16
>> +#define AD7298_INTREF_mV	2500
>> +
>> +#define AD7298_CH_TEMP		9
>> +
>> +#define RES_MASK(bits)	((1 << (bits)) - 1)
>> +
>> +struct ad7298_state {
>> +	struct spi_device		*spi;
>> +	struct regulator		*reg;
>> +	unsigned			ext_ref;
>> +	struct spi_transfer		ring_xfer[10];
>> +	struct spi_transfer		scan_single_xfer[3];
>> +	struct spi_message		ring_msg;
>> +	struct spi_message		scan_single_msg;
>> +	/*
>> +	 * DMA (thus cache coherency maintenance) requires the
>> +	 * transfer buffers to live in their own cache lines.
>> +	 */
>> +	unsigned short			rx_buf[12] ____cacheline_aligned;
>> +	unsigned short			tx_buf[2];
>> +};
>> +
>>  #define AD7298_V_CHAN(index)						\
>>  	{								\
>>  		.type = IIO_VOLTAGE,					\
>> @@ -66,6 +102,84 @@ static const struct iio_chan_spec ad7298_channels[] = {
>>  	IIO_CHAN_SOFT_TIMESTAMP(8),
>>  };
>>  
>> +/**
>> + * ad7298_update_scan_mode() setup the spi transfer buffer for the new scan mask
>> + **/
>> +static int ad7298_update_scan_mode(struct iio_dev *indio_dev,
>> +	const unsigned long *active_scan_mask)
>> +{
>> +	struct ad7298_state *st = iio_priv(indio_dev);
>> +	int i, m;
>> +	unsigned short command;
>> +	int scan_count;
>> +
>> +	/* Now compute overall size */
>> +	scan_count = bitmap_weight(active_scan_mask, indio_dev->masklength);
>> +
>> +	command = AD7298_WRITE | st->ext_ref;
>> +
>> +	for (i = 0, m = AD7298_CH(0); i < AD7298_MAX_CHAN; i++, m >>= 1)
>> +		if (test_bit(i, active_scan_mask))
>> +			command |= m;
>> +
>> +	st->tx_buf[0] = cpu_to_be16(command);
>> +
>> +	/* build spi ring message */
>> +	st->ring_xfer[0].tx_buf = &st->tx_buf[0];
>> +	st->ring_xfer[0].len = 2;
>> +	st->ring_xfer[0].cs_change = 1;
>> +	st->ring_xfer[1].tx_buf = &st->tx_buf[1];
>> +	st->ring_xfer[1].len = 2;
>> +	st->ring_xfer[1].cs_change = 1;
>> +
>> +	spi_message_init(&st->ring_msg);
>> +	spi_message_add_tail(&st->ring_xfer[0], &st->ring_msg);
>> +	spi_message_add_tail(&st->ring_xfer[1], &st->ring_msg);
>> +
>> +	for (i = 0; i < scan_count; i++) {
>> +		st->ring_xfer[i + 2].rx_buf = &st->rx_buf[i];
>> +		st->ring_xfer[i + 2].len = 2;
>> +		st->ring_xfer[i + 2].cs_change = 1;
>> +		spi_message_add_tail(&st->ring_xfer[i + 2], &st->ring_msg);
>> +	}
>> +	/* make sure last transfer cs_change is not set */
>> +	st->ring_xfer[i + 1].cs_change = 0;
>> +
>> +	return 0;
>> +}
>> +
>> +/**
>> + * ad7298_trigger_handler() bh of trigger launched polling to ring buffer
>> + *
>> + * Currently there is no option in this driver to disable the saving of
>> + * timestamps within the ring.
>> + **/
>> +static irqreturn_t ad7298_trigger_handler(int irq, void *p)
>> +{
>> +	struct iio_poll_func *pf = p;
>> +	struct iio_dev *indio_dev = pf->indio_dev;
>> +	struct ad7298_state *st = iio_priv(indio_dev);
>> +	s64 time_ns = 0;
>> +	int b_sent;
>> +
>> +	b_sent = spi_sync(st->spi, &st->ring_msg);
>> +	if (b_sent)
>> +		goto done;
>> +
>> +	if (indio_dev->scan_timestamp) {
>> +		time_ns = iio_get_time_ns();
>> +		memcpy((u8 *)st->rx_buf + indio_dev->scan_bytes - sizeof(s64),
>> +			&time_ns, sizeof(time_ns));
>> +	}
>> +
>> +	iio_push_to_buffers(indio_dev, (u8 *)st->rx_buf);
>> +
>> +done:
>> +	iio_trigger_notify_done(indio_dev->trig);
>> +
>> +	return IRQ_HANDLED;
>> +}
>> +
>>  static int ad7298_scan_direct(struct ad7298_state *st, unsigned ch)
>>  {
>>  	int ret;
>> @@ -231,7 +345,8 @@ static int __devinit ad7298_probe(struct spi_device *spi)
>>  	spi_message_add_tail(&st->scan_single_xfer[1], &st->scan_single_msg);
>>  	spi_message_add_tail(&st->scan_single_xfer[2], &st->scan_single_msg);
>>  
>> -	ret = ad7298_register_ring_funcs_and_init(indio_dev);
>> +	ret = iio_triggered_buffer_setup(indio_dev, NULL,
>> +			&ad7298_trigger_handler, NULL);
>>  	if (ret)
>>  		goto error_disable_reg;
>>  
>> @@ -242,7 +357,7 @@ static int __devinit ad7298_probe(struct spi_device *spi)
>>  	return 0;
>>  
>>  error_cleanup_ring:
>> -	ad7298_ring_cleanup(indio_dev);
>> +	iio_triggered_buffer_cleanup(indio_dev);
>>  error_disable_reg:
>>  	if (st->ext_ref)
>>  		regulator_disable(st->reg);
>> @@ -261,7 +376,7 @@ static int __devexit ad7298_remove(struct spi_device *spi)
>>  	struct ad7298_state *st = iio_priv(indio_dev);
>>  
>>  	iio_device_unregister(indio_dev);
>> -	ad7298_ring_cleanup(indio_dev);
>> +	iio_triggered_buffer_cleanup(indio_dev);
>>  	if (st->ext_ref) {
>>  		regulator_disable(st->reg);
>>  		regulator_put(st->reg);
>> diff --git a/drivers/staging/iio/adc/ad7298.h b/drivers/staging/iio/adc/ad7298.h
>> index 6523e01..c8ac969 100644
>> --- a/drivers/staging/iio/adc/ad7298.h
>> +++ b/drivers/staging/iio/adc/ad7298.h
>> @@ -9,23 +9,6 @@
>>  #ifndef IIO_ADC_AD7298_H_
>>  #define IIO_ADC_AD7298_H_
>>  
>> -#define AD7298_WRITE	(1 << 15) /* write to the control register */
>> -#define AD7298_REPEAT	(1 << 14) /* repeated conversion enable */
>> -#define AD7298_CH(x)	(1 << (13 - (x))) /* channel select */
>> -#define AD7298_TSENSE	(1 << 5) /* temperature conversion enable */
>> -#define AD7298_EXTREF	(1 << 2) /* external reference enable */
>> -#define AD7298_TAVG	(1 << 1) /* temperature sensor averaging enable */
>> -#define AD7298_PDD	(1 << 0) /* partial power down enable */
>> -
>> -#define AD7298_MAX_CHAN		8
>> -#define AD7298_BITS		12
>> -#define AD7298_STORAGE_BITS	16
>> -#define AD7298_INTREF_mV	2500
>> -
>> -#define AD7298_CH_TEMP		9
>> -
>> -#define RES_MASK(bits)	((1 << (bits)) - 1)
>> -
>>  /**
>>   * struct ad7298_platform_data - Platform data for the ad7298 ADC driver
>>   * @ext_ref: Whether to use an external reference voltage.
>> @@ -34,40 +17,4 @@ struct ad7298_platform_data {
>>  	bool ext_ref;
>>  };
>>  
>> -struct ad7298_state {
>> -	struct spi_device		*spi;
>> -	struct regulator		*reg;
>> -	unsigned			ext_ref;
>> -	struct spi_transfer		ring_xfer[10];
>> -	struct spi_transfer		scan_single_xfer[3];
>> -	struct spi_message		ring_msg;
>> -	struct spi_message		scan_single_msg;
>> -	/*
>> -	 * DMA (thus cache coherency maintenance) requires the
>> -	 * transfer buffers to live in their own cache lines.
>> -	 */
>> -	unsigned short			rx_buf[12] ____cacheline_aligned;
>> -	unsigned short			tx_buf[2];
>> -};
>> -
>> -#ifdef CONFIG_IIO_BUFFER
>> -int ad7298_register_ring_funcs_and_init(struct iio_dev *indio_dev);
>> -void ad7298_ring_cleanup(struct iio_dev *indio_dev);
>> -int ad7298_update_scan_mode(struct iio_dev *indio_dev,
>> -	const unsigned long *active_scan_mask);
>> -#else /* CONFIG_IIO_BUFFER */
>> -
>> -static inline int
>> -ad7298_register_ring_funcs_and_init(struct iio_dev *indio_dev)
>> -{
>> -	return 0;
>> -}
>> -
>> -static inline void ad7298_ring_cleanup(struct iio_dev *indio_dev)
>> -{
>> -}
>> -
>> -#define ad7298_update_scan_mode NULL
>> -
>> -#endif /* CONFIG_IIO_BUFFER */
>>  #endif /* IIO_ADC_AD7298_H_ */
>> diff --git a/drivers/staging/iio/adc/ad7298_ring.c b/drivers/staging/iio/adc/ad7298_ring.c
>> deleted file mode 100644
>> index e387712..0000000
>> --- a/drivers/staging/iio/adc/ad7298_ring.c
>> +++ /dev/null
>> @@ -1,108 +0,0 @@
>> -/*
>> - * AD7298 SPI ADC driver
>> - *
>> - * Copyright 2011-2012 Analog Devices Inc.
>> - *
>> - * Licensed under the GPL-2.
>> - */
>> -
>> -#include <linux/interrupt.h>
>> -#include <linux/kernel.h>
>> -#include <linux/slab.h>
>> -#include <linux/spi/spi.h>
>> -
>> -#include <linux/iio/iio.h>
>> -#include <linux/iio/buffer.h>
>> -#include <linux/iio/trigger_consumer.h>
>> -#include <linux/iio/triggered_buffer.h>
>> -
>> -#include "ad7298.h"
>> -
>> -/**
>> - * ad7298_update_scan_mode() setup the spi transfer buffer for the new scan mask
>> - **/
>> -int ad7298_update_scan_mode(struct iio_dev *indio_dev,
>> -	const unsigned long *active_scan_mask)
>> -{
>> -	struct ad7298_state *st = iio_priv(indio_dev);
>> -	int i, m;
>> -	unsigned short command;
>> -	int scan_count;
>> -
>> -	/* Now compute overall size */
>> -	scan_count = bitmap_weight(active_scan_mask, indio_dev->masklength);
>> -
>> -	command = AD7298_WRITE | st->ext_ref;
>> -
>> -	for (i = 0, m = AD7298_CH(0); i < AD7298_MAX_CHAN; i++, m >>= 1)
>> -		if (test_bit(i, active_scan_mask))
>> -			command |= m;
>> -
>> -	st->tx_buf[0] = cpu_to_be16(command);
>> -
>> -	/* build spi ring message */
>> -	st->ring_xfer[0].tx_buf = &st->tx_buf[0];
>> -	st->ring_xfer[0].len = 2;
>> -	st->ring_xfer[0].cs_change = 1;
>> -	st->ring_xfer[1].tx_buf = &st->tx_buf[1];
>> -	st->ring_xfer[1].len = 2;
>> -	st->ring_xfer[1].cs_change = 1;
>> -
>> -	spi_message_init(&st->ring_msg);
>> -	spi_message_add_tail(&st->ring_xfer[0], &st->ring_msg);
>> -	spi_message_add_tail(&st->ring_xfer[1], &st->ring_msg);
>> -
>> -	for (i = 0; i < scan_count; i++) {
>> -		st->ring_xfer[i + 2].rx_buf = &st->rx_buf[i];
>> -		st->ring_xfer[i + 2].len = 2;
>> -		st->ring_xfer[i + 2].cs_change = 1;
>> -		spi_message_add_tail(&st->ring_xfer[i + 2], &st->ring_msg);
>> -	}
>> -	/* make sure last transfer cs_change is not set */
>> -	st->ring_xfer[i + 1].cs_change = 0;
>> -
>> -	return 0;
>> -}
>> -
>> -/**
>> - * ad7298_trigger_handler() bh of trigger launched polling to ring buffer
>> - *
>> - * Currently there is no option in this driver to disable the saving of
>> - * timestamps within the ring.
>> - **/
>> -static irqreturn_t ad7298_trigger_handler(int irq, void *p)
>> -{
>> -	struct iio_poll_func *pf = p;
>> -	struct iio_dev *indio_dev = pf->indio_dev;
>> -	struct ad7298_state *st = iio_priv(indio_dev);
>> -	s64 time_ns = 0;
>> -	int b_sent;
>> -
>> -	b_sent = spi_sync(st->spi, &st->ring_msg);
>> -	if (b_sent)
>> -		goto done;
>> -
>> -	if (indio_dev->scan_timestamp) {
>> -		time_ns = iio_get_time_ns();
>> -		memcpy((u8 *)st->rx_buf + indio_dev->scan_bytes - sizeof(s64),
>> -			&time_ns, sizeof(time_ns));
>> -	}
>> -
>> -	iio_push_to_buffers(indio_dev, (u8 *)st->rx_buf);
>> -
>> -done:
>> -	iio_trigger_notify_done(indio_dev->trig);
>> -
>> -	return IRQ_HANDLED;
>> -}
>> -
>> -int ad7298_register_ring_funcs_and_init(struct iio_dev *indio_dev)
>> -{
>> -	return iio_triggered_buffer_setup(indio_dev, NULL,
>> -			&ad7298_trigger_handler, NULL);
>> -}
>> -
>> -void ad7298_ring_cleanup(struct iio_dev *indio_dev)
>> -{
>> -	iio_triggered_buffer_cleanup(indio_dev);
>> -}
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 5/5] staging:iio: Move the ad7298 driver out of staging
  2012-11-15 13:15 ` [PATCH 5/5] staging:iio: Move the ad7298 driver out of staging Lars-Peter Clausen
@ 2012-11-17 11:45   ` Jonathan Cameron
  0 siblings, 0 replies; 12+ messages in thread
From: Jonathan Cameron @ 2012-11-17 11:45 UTC (permalink / raw)
  To: Lars-Peter Clausen; +Cc: Jonathan Cameron, linux-iio, drivers

On 11/15/2012 01:15 PM, Lars-Peter Clausen wrote:
> The driver does not expose any custom API to userspace and none of the standard
> static code checker tools report any issues, so move it out of staging.
> 
one day I'll work out how to make a tiny change in an earlier patch without needing
to respin the entire follow up more or less by hand.

Anyhow, now done, and added to togreg branch of iio.git

Other than that little slip, a nice patch set Lars.
Thanks!
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> ---
>  drivers/iio/adc/Kconfig                                      | 12 ++++++++++++
>  drivers/iio/adc/Makefile                                     |  1 +
>  drivers/{staging => }/iio/adc/ad7298.c                       |  2 +-
>  drivers/staging/iio/adc/Kconfig                              | 12 ------------
>  drivers/staging/iio/adc/Makefile                             |  1 -
>  .../staging/iio/adc => include/linux/platform_data}/ad7298.h |  4 ++--
>  6 files changed, 16 insertions(+), 16 deletions(-)
>  rename drivers/{staging => }/iio/adc/ad7298.c (99%)
>  rename {drivers/staging/iio/adc => include/linux/platform_data}/ad7298.h (80%)
> 
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index ef5200a..cd5eed6 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -18,6 +18,18 @@ config AD7266
>  	  Say yes here to build support for Analog Devices AD7265 and AD7266
>  	  ADCs.
>  
> +config AD7298
> +	tristate "Analog Devices AD7298 ADC driver"
> +	depends on SPI
> +	select IIO_BUFFER
> +	select IIO_TRIGGERED_BUFFER
> +	help
> +	  Say yes here to build support for Analog Devices AD7298
> +	  8 Channel ADC with temperature sensor.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called ad7298.
> +
>  config AD7791
>  	tristate "Analog Devices AD7791 ADC driver"
>  	depends on SPI
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index 54ac7bb..3256dc6 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -4,6 +4,7 @@
>  
>  obj-$(CONFIG_AD_SIGMA_DELTA) += ad_sigma_delta.o
>  obj-$(CONFIG_AD7266) += ad7266.o
> +obj-$(CONFIG_AD7298) += ad7298.o
>  obj-$(CONFIG_AD7476) += ad7476.o
>  obj-$(CONFIG_AD7791) += ad7791.o
>  obj-$(CONFIG_AD7887) += ad7887.o
> diff --git a/drivers/staging/iio/adc/ad7298.c b/drivers/iio/adc/ad7298.c
> similarity index 99%
> rename from drivers/staging/iio/adc/ad7298.c
> rename to drivers/iio/adc/ad7298.c
> index 2742a9d..441a9a2 100644
> --- a/drivers/staging/iio/adc/ad7298.c
> +++ b/drivers/iio/adc/ad7298.c
> @@ -23,7 +23,7 @@
>  #include <linux/iio/trigger_consumer.h>
>  #include <linux/iio/triggered_buffer.h>
>  
> -#include "ad7298.h"
> +#include <linux/platform_data/ad7298.h>
>  
>  #define AD7298_WRITE	(1 << 15) /* write to the control register */
>  #define AD7298_REPEAT	(1 << 14) /* repeated conversion enable */
> diff --git a/drivers/staging/iio/adc/Kconfig b/drivers/staging/iio/adc/Kconfig
> index 5086a46..dc8582b 100644
> --- a/drivers/staging/iio/adc/Kconfig
> +++ b/drivers/staging/iio/adc/Kconfig
> @@ -10,18 +10,6 @@ config AD7291
>  	  Say yes here to build support for Analog Devices AD7291
>  	  8 Channel ADC with temperature sensor.
>  
> -config AD7298
> -	tristate "Analog Devices AD7298 ADC driver"
> -	depends on SPI
> -	select IIO_BUFFER
> -	select IIO_TRIGGERED_BUFFER
> -	help
> -	  Say yes here to build support for Analog Devices AD7298
> -	  8 Channel ADC with temperature sensor.
> -
> -	  To compile this driver as a module, choose M here: the
> -	  module will be called ad7298.
> -
>  config AD7606
>  	tristate "Analog Devices AD7606 ADC driver"
>  	depends on GPIOLIB
> diff --git a/drivers/staging/iio/adc/Makefile b/drivers/staging/iio/adc/Makefile
> index e867d01..a80e603 100644
> --- a/drivers/staging/iio/adc/Makefile
> +++ b/drivers/staging/iio/adc/Makefile
> @@ -13,7 +13,6 @@ ad799x-$(CONFIG_AD799X_RING_BUFFER) += ad799x_ring.o
>  obj-$(CONFIG_AD799X) += ad799x.o
>  
>  obj-$(CONFIG_AD7887) += ad7887.o
> -obj-$(CONFIG_AD7298) += ad7298.o
>  
>  obj-$(CONFIG_AD7291) += ad7291.o
>  obj-$(CONFIG_AD7780) += ad7780.o
> diff --git a/drivers/staging/iio/adc/ad7298.h b/include/linux/platform_data/ad7298.h
> similarity index 80%
> rename from drivers/staging/iio/adc/ad7298.h
> rename to include/linux/platform_data/ad7298.h
> index c8ac969..fbf8adf 100644
> --- a/drivers/staging/iio/adc/ad7298.h
> +++ b/include/linux/platform_data/ad7298.h
> @@ -6,8 +6,8 @@
>   * Licensed under the GPL-2.
>   */
>  
> -#ifndef IIO_ADC_AD7298_H_
> -#define IIO_ADC_AD7298_H_
> +#ifndef __LINUX_PLATFORM_DATA_AD7298_H__
> +#define __LINUX_PLATFORM_DATA_AD7298_H__
>  
>  /**
>   * struct ad7298_platform_data - Platform data for the ad7298 ADC driver
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 4/5] staging:iio:ad7298: Squash everything into one file
  2012-11-17 11:34     ` Jonathan Cameron
@ 2012-11-17 17:23       ` Lars-Peter Clausen
  0 siblings, 0 replies; 12+ messages in thread
From: Lars-Peter Clausen @ 2012-11-17 17:23 UTC (permalink / raw)
  To: Jonathan Cameron; +Cc: Jonathan Cameron, linux-iio, drivers

On 11/17/2012 12:34 PM, Jonathan Cameron wrote:
> On 11/17/2012 11:20 AM, Jonathan Cameron wrote:
>> On 11/15/2012 01:15 PM, Lars-Peter Clausen wrote:
>>> The recent cleanups have decimated the drivers code size by quite a bit. It is
>>> only a few hundred lines in total now. Putting everything into one file also
>>> allows to reduce the code size a bit more by removing a few lines of boilerplate
>>> code. The only functional change made by this patch is that we now always
>>> include buffer support, instead of making it optional. This is more consistent
>>> with what we do for other drivers.
>>>
>> added to togreg branch of iio.git
> Actually scratch that - my build test hadn't finished and I jumped the gun.
> 
> You have a reference to ad7887 bleeding in from somewhere in the makefile.
> I've fixed it up before pushing out

Ah, damm, that must have slipped in during the rebase onto staging-next.
Thanks for taking care of this and of course also for taking the time to review
and apply all the patch :)

> 
> 
>>> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
>>> ---
>>>  drivers/staging/iio/adc/Kconfig                    |   3 +-
>>>  drivers/staging/iio/adc/Makefile                   |   3 +-
>>>  .../staging/iio/adc/{ad7298_core.c => ad7298.c}    | 121 ++++++++++++++++++++-
>>>  drivers/staging/iio/adc/ad7298.h                   |  53 ---------
>>>  drivers/staging/iio/adc/ad7298_ring.c              | 108 ------------------
>>>  5 files changed, 121 insertions(+), 167 deletions(-)
>>>  rename drivers/staging/iio/adc/{ad7298_core.c => ad7298.c} (64%)
>>>  delete mode 100644 drivers/staging/iio/adc/ad7298_ring.c
>>>
>>> diff --git a/drivers/staging/iio/adc/Kconfig b/drivers/staging/iio/adc/Kconfig
>>> index 0177f1e..5086a46 100644
>>> --- a/drivers/staging/iio/adc/Kconfig
>>> +++ b/drivers/staging/iio/adc/Kconfig
>>> @@ -13,7 +13,8 @@ config AD7291
>>>  config AD7298
>>>  	tristate "Analog Devices AD7298 ADC driver"
>>>  	depends on SPI
>>> -	select IIO_TRIGGERED_BUFFER if IIO_BUFFER
>>> +	select IIO_BUFFER
>>> +	select IIO_TRIGGERED_BUFFER
>>>  	help
>>>  	  Say yes here to build support for Analog Devices AD7298
>>>  	  8 Channel ADC with temperature sensor.
>>> diff --git a/drivers/staging/iio/adc/Makefile b/drivers/staging/iio/adc/Makefile
>>> index 12b4bd3..e867d01 100644
>>> --- a/drivers/staging/iio/adc/Makefile
>>> +++ b/drivers/staging/iio/adc/Makefile
>>> @@ -12,8 +12,7 @@ ad799x-y := ad799x_core.o
>>>  ad799x-$(CONFIG_AD799X_RING_BUFFER) += ad799x_ring.o
>>>  obj-$(CONFIG_AD799X) += ad799x.o
>>>  
>>> -ad7298-y := ad7298_core.o
>>> -ad7298-$(CONFIG_IIO_BUFFER) += ad7298_ring.o
>>> +obj-$(CONFIG_AD7887) += ad7887.o
>>>  obj-$(CONFIG_AD7298) += ad7298.o
>>>  
>>>  obj-$(CONFIG_AD7291) += ad7291.o
>>> diff --git a/drivers/staging/iio/adc/ad7298_core.c b/drivers/staging/iio/adc/ad7298.c
>>> similarity index 64%
>>> rename from drivers/staging/iio/adc/ad7298_core.c
>>> rename to drivers/staging/iio/adc/ad7298.c
>>> index b74b76f..2742a9d 100644
>>> --- a/drivers/staging/iio/adc/ad7298_core.c
>>> +++ b/drivers/staging/iio/adc/ad7298.c
>>> @@ -15,13 +15,49 @@
>>>  #include <linux/err.h>
>>>  #include <linux/delay.h>
>>>  #include <linux/module.h>
>>> +#include <linux/interrupt.h>
>>>  
>>>  #include <linux/iio/iio.h>
>>>  #include <linux/iio/sysfs.h>
>>>  #include <linux/iio/buffer.h>
>>> +#include <linux/iio/trigger_consumer.h>
>>> +#include <linux/iio/triggered_buffer.h>
>>>  
>>>  #include "ad7298.h"
>>>  
>>> +#define AD7298_WRITE	(1 << 15) /* write to the control register */
>>> +#define AD7298_REPEAT	(1 << 14) /* repeated conversion enable */
>>> +#define AD7298_CH(x)	(1 << (13 - (x))) /* channel select */
>>> +#define AD7298_TSENSE	(1 << 5) /* temperature conversion enable */
>>> +#define AD7298_EXTREF	(1 << 2) /* external reference enable */
>>> +#define AD7298_TAVG	(1 << 1) /* temperature sensor averaging enable */
>>> +#define AD7298_PDD	(1 << 0) /* partial power down enable */
>>> +
>>> +#define AD7298_MAX_CHAN		8
>>> +#define AD7298_BITS		12
>>> +#define AD7298_STORAGE_BITS	16
>>> +#define AD7298_INTREF_mV	2500
>>> +
>>> +#define AD7298_CH_TEMP		9
>>> +
>>> +#define RES_MASK(bits)	((1 << (bits)) - 1)
>>> +
>>> +struct ad7298_state {
>>> +	struct spi_device		*spi;
>>> +	struct regulator		*reg;
>>> +	unsigned			ext_ref;
>>> +	struct spi_transfer		ring_xfer[10];
>>> +	struct spi_transfer		scan_single_xfer[3];
>>> +	struct spi_message		ring_msg;
>>> +	struct spi_message		scan_single_msg;
>>> +	/*
>>> +	 * DMA (thus cache coherency maintenance) requires the
>>> +	 * transfer buffers to live in their own cache lines.
>>> +	 */
>>> +	unsigned short			rx_buf[12] ____cacheline_aligned;
>>> +	unsigned short			tx_buf[2];
>>> +};
>>> +
>>>  #define AD7298_V_CHAN(index)						\
>>>  	{								\
>>>  		.type = IIO_VOLTAGE,					\
>>> @@ -66,6 +102,84 @@ static const struct iio_chan_spec ad7298_channels[] = {
>>>  	IIO_CHAN_SOFT_TIMESTAMP(8),
>>>  };
>>>  
>>> +/**
>>> + * ad7298_update_scan_mode() setup the spi transfer buffer for the new scan mask
>>> + **/
>>> +static int ad7298_update_scan_mode(struct iio_dev *indio_dev,
>>> +	const unsigned long *active_scan_mask)
>>> +{
>>> +	struct ad7298_state *st = iio_priv(indio_dev);
>>> +	int i, m;
>>> +	unsigned short command;
>>> +	int scan_count;
>>> +
>>> +	/* Now compute overall size */
>>> +	scan_count = bitmap_weight(active_scan_mask, indio_dev->masklength);
>>> +
>>> +	command = AD7298_WRITE | st->ext_ref;
>>> +
>>> +	for (i = 0, m = AD7298_CH(0); i < AD7298_MAX_CHAN; i++, m >>= 1)
>>> +		if (test_bit(i, active_scan_mask))
>>> +			command |= m;
>>> +
>>> +	st->tx_buf[0] = cpu_to_be16(command);
>>> +
>>> +	/* build spi ring message */
>>> +	st->ring_xfer[0].tx_buf = &st->tx_buf[0];
>>> +	st->ring_xfer[0].len = 2;
>>> +	st->ring_xfer[0].cs_change = 1;
>>> +	st->ring_xfer[1].tx_buf = &st->tx_buf[1];
>>> +	st->ring_xfer[1].len = 2;
>>> +	st->ring_xfer[1].cs_change = 1;
>>> +
>>> +	spi_message_init(&st->ring_msg);
>>> +	spi_message_add_tail(&st->ring_xfer[0], &st->ring_msg);
>>> +	spi_message_add_tail(&st->ring_xfer[1], &st->ring_msg);
>>> +
>>> +	for (i = 0; i < scan_count; i++) {
>>> +		st->ring_xfer[i + 2].rx_buf = &st->rx_buf[i];
>>> +		st->ring_xfer[i + 2].len = 2;
>>> +		st->ring_xfer[i + 2].cs_change = 1;
>>> +		spi_message_add_tail(&st->ring_xfer[i + 2], &st->ring_msg);
>>> +	}
>>> +	/* make sure last transfer cs_change is not set */
>>> +	st->ring_xfer[i + 1].cs_change = 0;
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +/**
>>> + * ad7298_trigger_handler() bh of trigger launched polling to ring buffer
>>> + *
>>> + * Currently there is no option in this driver to disable the saving of
>>> + * timestamps within the ring.
>>> + **/
>>> +static irqreturn_t ad7298_trigger_handler(int irq, void *p)
>>> +{
>>> +	struct iio_poll_func *pf = p;
>>> +	struct iio_dev *indio_dev = pf->indio_dev;
>>> +	struct ad7298_state *st = iio_priv(indio_dev);
>>> +	s64 time_ns = 0;
>>> +	int b_sent;
>>> +
>>> +	b_sent = spi_sync(st->spi, &st->ring_msg);
>>> +	if (b_sent)
>>> +		goto done;
>>> +
>>> +	if (indio_dev->scan_timestamp) {
>>> +		time_ns = iio_get_time_ns();
>>> +		memcpy((u8 *)st->rx_buf + indio_dev->scan_bytes - sizeof(s64),
>>> +			&time_ns, sizeof(time_ns));
>>> +	}
>>> +
>>> +	iio_push_to_buffers(indio_dev, (u8 *)st->rx_buf);
>>> +
>>> +done:
>>> +	iio_trigger_notify_done(indio_dev->trig);
>>> +
>>> +	return IRQ_HANDLED;
>>> +}
>>> +
>>>  static int ad7298_scan_direct(struct ad7298_state *st, unsigned ch)
>>>  {
>>>  	int ret;
>>> @@ -231,7 +345,8 @@ static int __devinit ad7298_probe(struct spi_device *spi)
>>>  	spi_message_add_tail(&st->scan_single_xfer[1], &st->scan_single_msg);
>>>  	spi_message_add_tail(&st->scan_single_xfer[2], &st->scan_single_msg);
>>>  
>>> -	ret = ad7298_register_ring_funcs_and_init(indio_dev);
>>> +	ret = iio_triggered_buffer_setup(indio_dev, NULL,
>>> +			&ad7298_trigger_handler, NULL);
>>>  	if (ret)
>>>  		goto error_disable_reg;
>>>  
>>> @@ -242,7 +357,7 @@ static int __devinit ad7298_probe(struct spi_device *spi)
>>>  	return 0;
>>>  
>>>  error_cleanup_ring:
>>> -	ad7298_ring_cleanup(indio_dev);
>>> +	iio_triggered_buffer_cleanup(indio_dev);
>>>  error_disable_reg:
>>>  	if (st->ext_ref)
>>>  		regulator_disable(st->reg);
>>> @@ -261,7 +376,7 @@ static int __devexit ad7298_remove(struct spi_device *spi)
>>>  	struct ad7298_state *st = iio_priv(indio_dev);
>>>  
>>>  	iio_device_unregister(indio_dev);
>>> -	ad7298_ring_cleanup(indio_dev);
>>> +	iio_triggered_buffer_cleanup(indio_dev);
>>>  	if (st->ext_ref) {
>>>  		regulator_disable(st->reg);
>>>  		regulator_put(st->reg);
>>> diff --git a/drivers/staging/iio/adc/ad7298.h b/drivers/staging/iio/adc/ad7298.h
>>> index 6523e01..c8ac969 100644
>>> --- a/drivers/staging/iio/adc/ad7298.h
>>> +++ b/drivers/staging/iio/adc/ad7298.h
>>> @@ -9,23 +9,6 @@
>>>  #ifndef IIO_ADC_AD7298_H_
>>>  #define IIO_ADC_AD7298_H_
>>>  
>>> -#define AD7298_WRITE	(1 << 15) /* write to the control register */
>>> -#define AD7298_REPEAT	(1 << 14) /* repeated conversion enable */
>>> -#define AD7298_CH(x)	(1 << (13 - (x))) /* channel select */
>>> -#define AD7298_TSENSE	(1 << 5) /* temperature conversion enable */
>>> -#define AD7298_EXTREF	(1 << 2) /* external reference enable */
>>> -#define AD7298_TAVG	(1 << 1) /* temperature sensor averaging enable */
>>> -#define AD7298_PDD	(1 << 0) /* partial power down enable */
>>> -
>>> -#define AD7298_MAX_CHAN		8
>>> -#define AD7298_BITS		12
>>> -#define AD7298_STORAGE_BITS	16
>>> -#define AD7298_INTREF_mV	2500
>>> -
>>> -#define AD7298_CH_TEMP		9
>>> -
>>> -#define RES_MASK(bits)	((1 << (bits)) - 1)
>>> -
>>>  /**
>>>   * struct ad7298_platform_data - Platform data for the ad7298 ADC driver
>>>   * @ext_ref: Whether to use an external reference voltage.
>>> @@ -34,40 +17,4 @@ struct ad7298_platform_data {
>>>  	bool ext_ref;
>>>  };
>>>  
>>> -struct ad7298_state {
>>> -	struct spi_device		*spi;
>>> -	struct regulator		*reg;
>>> -	unsigned			ext_ref;
>>> -	struct spi_transfer		ring_xfer[10];
>>> -	struct spi_transfer		scan_single_xfer[3];
>>> -	struct spi_message		ring_msg;
>>> -	struct spi_message		scan_single_msg;
>>> -	/*
>>> -	 * DMA (thus cache coherency maintenance) requires the
>>> -	 * transfer buffers to live in their own cache lines.
>>> -	 */
>>> -	unsigned short			rx_buf[12] ____cacheline_aligned;
>>> -	unsigned short			tx_buf[2];
>>> -};
>>> -
>>> -#ifdef CONFIG_IIO_BUFFER
>>> -int ad7298_register_ring_funcs_and_init(struct iio_dev *indio_dev);
>>> -void ad7298_ring_cleanup(struct iio_dev *indio_dev);
>>> -int ad7298_update_scan_mode(struct iio_dev *indio_dev,
>>> -	const unsigned long *active_scan_mask);
>>> -#else /* CONFIG_IIO_BUFFER */
>>> -
>>> -static inline int
>>> -ad7298_register_ring_funcs_and_init(struct iio_dev *indio_dev)
>>> -{
>>> -	return 0;
>>> -}
>>> -
>>> -static inline void ad7298_ring_cleanup(struct iio_dev *indio_dev)
>>> -{
>>> -}
>>> -
>>> -#define ad7298_update_scan_mode NULL
>>> -
>>> -#endif /* CONFIG_IIO_BUFFER */
>>>  #endif /* IIO_ADC_AD7298_H_ */
>>> diff --git a/drivers/staging/iio/adc/ad7298_ring.c b/drivers/staging/iio/adc/ad7298_ring.c
>>> deleted file mode 100644
>>> index e387712..0000000
>>> --- a/drivers/staging/iio/adc/ad7298_ring.c
>>> +++ /dev/null
>>> @@ -1,108 +0,0 @@
>>> -/*
>>> - * AD7298 SPI ADC driver
>>> - *
>>> - * Copyright 2011-2012 Analog Devices Inc.
>>> - *
>>> - * Licensed under the GPL-2.
>>> - */
>>> -
>>> -#include <linux/interrupt.h>
>>> -#include <linux/kernel.h>
>>> -#include <linux/slab.h>
>>> -#include <linux/spi/spi.h>
>>> -
>>> -#include <linux/iio/iio.h>
>>> -#include <linux/iio/buffer.h>
>>> -#include <linux/iio/trigger_consumer.h>
>>> -#include <linux/iio/triggered_buffer.h>
>>> -
>>> -#include "ad7298.h"
>>> -
>>> -/**
>>> - * ad7298_update_scan_mode() setup the spi transfer buffer for the new scan mask
>>> - **/
>>> -int ad7298_update_scan_mode(struct iio_dev *indio_dev,
>>> -	const unsigned long *active_scan_mask)
>>> -{
>>> -	struct ad7298_state *st = iio_priv(indio_dev);
>>> -	int i, m;
>>> -	unsigned short command;
>>> -	int scan_count;
>>> -
>>> -	/* Now compute overall size */
>>> -	scan_count = bitmap_weight(active_scan_mask, indio_dev->masklength);
>>> -
>>> -	command = AD7298_WRITE | st->ext_ref;
>>> -
>>> -	for (i = 0, m = AD7298_CH(0); i < AD7298_MAX_CHAN; i++, m >>= 1)
>>> -		if (test_bit(i, active_scan_mask))
>>> -			command |= m;
>>> -
>>> -	st->tx_buf[0] = cpu_to_be16(command);
>>> -
>>> -	/* build spi ring message */
>>> -	st->ring_xfer[0].tx_buf = &st->tx_buf[0];
>>> -	st->ring_xfer[0].len = 2;
>>> -	st->ring_xfer[0].cs_change = 1;
>>> -	st->ring_xfer[1].tx_buf = &st->tx_buf[1];
>>> -	st->ring_xfer[1].len = 2;
>>> -	st->ring_xfer[1].cs_change = 1;
>>> -
>>> -	spi_message_init(&st->ring_msg);
>>> -	spi_message_add_tail(&st->ring_xfer[0], &st->ring_msg);
>>> -	spi_message_add_tail(&st->ring_xfer[1], &st->ring_msg);
>>> -
>>> -	for (i = 0; i < scan_count; i++) {
>>> -		st->ring_xfer[i + 2].rx_buf = &st->rx_buf[i];
>>> -		st->ring_xfer[i + 2].len = 2;
>>> -		st->ring_xfer[i + 2].cs_change = 1;
>>> -		spi_message_add_tail(&st->ring_xfer[i + 2], &st->ring_msg);
>>> -	}
>>> -	/* make sure last transfer cs_change is not set */
>>> -	st->ring_xfer[i + 1].cs_change = 0;
>>> -
>>> -	return 0;
>>> -}
>>> -
>>> -/**
>>> - * ad7298_trigger_handler() bh of trigger launched polling to ring buffer
>>> - *
>>> - * Currently there is no option in this driver to disable the saving of
>>> - * timestamps within the ring.
>>> - **/
>>> -static irqreturn_t ad7298_trigger_handler(int irq, void *p)
>>> -{
>>> -	struct iio_poll_func *pf = p;
>>> -	struct iio_dev *indio_dev = pf->indio_dev;
>>> -	struct ad7298_state *st = iio_priv(indio_dev);
>>> -	s64 time_ns = 0;
>>> -	int b_sent;
>>> -
>>> -	b_sent = spi_sync(st->spi, &st->ring_msg);
>>> -	if (b_sent)
>>> -		goto done;
>>> -
>>> -	if (indio_dev->scan_timestamp) {
>>> -		time_ns = iio_get_time_ns();
>>> -		memcpy((u8 *)st->rx_buf + indio_dev->scan_bytes - sizeof(s64),
>>> -			&time_ns, sizeof(time_ns));
>>> -	}
>>> -
>>> -	iio_push_to_buffers(indio_dev, (u8 *)st->rx_buf);
>>> -
>>> -done:
>>> -	iio_trigger_notify_done(indio_dev->trig);
>>> -
>>> -	return IRQ_HANDLED;
>>> -}
>>> -
>>> -int ad7298_register_ring_funcs_and_init(struct iio_dev *indio_dev)
>>> -{
>>> -	return iio_triggered_buffer_setup(indio_dev, NULL,
>>> -			&ad7298_trigger_handler, NULL);
>>> -}
>>> -
>>> -void ad7298_ring_cleanup(struct iio_dev *indio_dev)
>>> -{
>>> -	iio_triggered_buffer_cleanup(indio_dev);
>>> -}
>>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2012-11-17 17:23 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-11-15 13:15 [PATCH 1/5] staging:iio:ad7298: Do not perform endianness conversion in buffered mode Lars-Peter Clausen
2012-11-15 13:15 ` [PATCH 2/5] staging:iio:ad7298: Rework regulator handling Lars-Peter Clausen
2012-11-17 11:17   ` Jonathan Cameron
2012-11-15 13:15 ` [PATCH 3/5] staging:iio:ad7298: Fix temperature scale and offset Lars-Peter Clausen
2012-11-17 11:19   ` Jonathan Cameron
2012-11-15 13:15 ` [PATCH 4/5] staging:iio:ad7298: Squash everything into one file Lars-Peter Clausen
2012-11-17 11:20   ` Jonathan Cameron
2012-11-17 11:34     ` Jonathan Cameron
2012-11-17 17:23       ` Lars-Peter Clausen
2012-11-15 13:15 ` [PATCH 5/5] staging:iio: Move the ad7298 driver out of staging Lars-Peter Clausen
2012-11-17 11:45   ` Jonathan Cameron
2012-11-17 11:16 ` [PATCH 1/5] staging:iio:ad7298: Do not perform endianness conversion in buffered mode Jonathan Cameron

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.