[PATCH 0/4] staging:iio:meter clean up and dead code removal.

[PATCH 0/4] staging:iio:meter clean up and dead code removal.

[Jonathan Cameron]

This is very much the starting point for bashing the meter drivers into
shape.  For now I've just removed most of the unused code and stubs.

I have also cleaned up spi bus handling.  I may well have gotten that
bit wrong so please do take a look.  In some cases this involved turning
one transfer into two so as to clarify what the code was doing.  This
may cause issues if the spi bus drivers don't play the game properly or
as if I got the conversion wrong.

Next job is to pin down, standardise and document the sysfs interface
for these parts.  Anyone care to take this on?

Note I have currently left the ade7758 driver alone
as it is by far the most complex and didn't suffer the large amount of
dead code to be found in the others.

All comments as ever welcome.  I'd also like to know what test
coverage we have on these parts.

Thanks,

Jonathan

Jonathan Cameron (4):
  staging:iio:meter remove stubs from ade7753.
  staging:iio:meter remove stubs from ade7754.
  staging:iio:meter remove stubs from ade7759.
  staging:iio:meter remove stubs from ade7854.

 drivers/staging/iio/meter/ade7753.c     |  200 +++++++------------------------
 drivers/staging/iio/meter/ade7753.h     |   64 ----------
 drivers/staging/iio/meter/ade7754.c     |  165 ++++---------------------
 drivers/staging/iio/meter/ade7754.h     |   67 ----------
 drivers/staging/iio/meter/ade7759.c     |  163 +++++---------------------
 drivers/staging/iio/meter/ade7759.h     |   65 ----------
 drivers/staging/iio/meter/ade7854-spi.c |  120 ++++++++++---------
 drivers/staging/iio/meter/ade7854.c     |   78 ++----------
 drivers/staging/iio/meter/ade7854.h     |   69 -----------
 9 files changed, 175 insertions(+), 816 deletions(-)

-- 
1.7.3.4

[PATCH 1/4] staging:iio:meter remove stubs from ade7753.

[Jonathan Cameron]

General cleanup and use of standard functions to simplfy some spi reads
as well.

Signed-off-by: Jonathan Cameron <jic23@cam.ac.uk>
---
 drivers/staging/iio/meter/ade7753.c |  200 ++++++++---------------------------
 drivers/staging/iio/meter/ade7753.h |   64 -----------
 2 files changed, 44 insertions(+), 220 deletions(-)

diff --git a/drivers/staging/iio/meter/ade7753.c b/drivers/staging/iio/meter/ade7753.c
index e72afbd..f57a350 100644
--- a/drivers/staging/iio/meter/ade7753.c
+++ b/drivers/staging/iio/meter/ade7753.c
@@ -1,5 +1,5 @@
 /*
- * ADE7753 Single-Phase Multifunction Metering IC with di/dt Sensor Interface Driver
+ * ADE7753 Single-Phase Multifunction Metering IC with di/dt Sensor Interface
  *
  * Copyright 2010 Analog Devices Inc.
  *
@@ -46,25 +46,14 @@ static int ade7753_spi_write_reg_16(struct device *dev,
 		u16 value)
 {
 	int ret;
-	struct spi_message msg;
 	struct iio_dev *indio_dev = dev_get_drvdata(dev);
 	struct ade7753_state *st = iio_dev_get_devdata(indio_dev);
-	struct spi_transfer xfers[] = {
-		{
-			.tx_buf = st->tx,
-			.bits_per_word = 8,
-			.len = 3,
-		}
-	};
 
 	mutex_lock(&st->buf_lock);
 	st->tx[0] = ADE7753_WRITE_REG(reg_address);
 	st->tx[1] = (value >> 8) & 0xFF;
 	st->tx[2] = value & 0xFF;
-
-	spi_message_init(&msg);
-	spi_message_add_tail(xfers, &msg);
-	ret = spi_sync(st->us, &msg);
+	ret = spi_write(st->us, st->tx, 3);
 	mutex_unlock(&st->buf_lock);
 
 	return ret;
@@ -74,73 +63,40 @@ static int ade7753_spi_read_reg_8(struct device *dev,
 		u8 reg_address,
 		u8 *val)
 {
-	struct spi_message msg;
 	struct iio_dev *indio_dev = dev_get_drvdata(dev);
 	struct ade7753_state *st = iio_dev_get_devdata(indio_dev);
-	int ret;
-	struct spi_transfer xfers[] = {
-		{
-			.tx_buf = st->tx,
-			.rx_buf = st->rx,
-			.bits_per_word = 8,
-			.len = 2,
-		},
-	};
-
-	mutex_lock(&st->buf_lock);
-	st->tx[0] = ADE7753_READ_REG(reg_address);
-	st->tx[1] = 0;
+	ssize_t ret;
 
-	spi_message_init(&msg);
-	spi_message_add_tail(xfers, &msg);
-	ret = spi_sync(st->us, &msg);
-	if (ret) {
+	ret = spi_w8r8(st->us, ADE7753_READ_REG(reg_address));
+	if (ret < 0) {
 		dev_err(&st->us->dev, "problem when reading 8 bit register 0x%02X",
 				reg_address);
-		goto error_ret;
+		return ret;
 	}
-	*val = st->rx[1];
+	*val = ret;
 
-error_ret:
-	mutex_unlock(&st->buf_lock);
-	return ret;
+	return 0;
 }
 
 static int ade7753_spi_read_reg_16(struct device *dev,
 		u8 reg_address,
 		u16 *val)
 {
-	struct spi_message msg;
 	struct iio_dev *indio_dev = dev_get_drvdata(dev);
 	struct ade7753_state *st = iio_dev_get_devdata(indio_dev);
-	int ret;
-	struct spi_transfer xfers[] = {
-		{
-			.tx_buf = st->tx,
-			.rx_buf = st->rx,
-			.bits_per_word = 8,
-			.len = 3,
-		},
-	};
-
-	mutex_lock(&st->buf_lock);
-	st->tx[0] = ADE7753_READ_REG(reg_address);
-	st->tx[1] = 0;
-	st->tx[2] = 0;
+	ssize_t ret;
 
-	spi_message_init(&msg);
-	spi_message_add_tail(xfers, &msg);
-	ret = spi_sync(st->us, &msg);
-	if (ret) {
+	ret = spi_w8r16(st->us, ADE7753_READ_REG(reg_address));
+	if (ret < 0) {
 		dev_err(&st->us->dev, "problem when reading 16 bit register 0x%02X",
-				reg_address);
-		goto error_ret;
+			reg_address);
+		return ret;
 	}
-	*val = (st->rx[1] << 8) | st->rx[2];
 
-error_ret:
-	mutex_unlock(&st->buf_lock);
-	return ret;
+	*val = ret;
+	*val = be16_to_cpup(val);
+
+	return 0;
 }
 
 static int ade7753_spi_read_reg_24(struct device *dev,
@@ -154,27 +110,28 @@ static int ade7753_spi_read_reg_24(struct device *dev,
 	struct spi_transfer xfers[] = {
 		{
 			.tx_buf = st->tx,
-			.rx_buf = st->rx,
 			.bits_per_word = 8,
-			.len = 4,
-		},
+			.len = 1,
+		}, {
+			.rx_buf = st->tx,
+			.bits_per_word = 8,
+			.len = 3,
+		}
 	};
 
 	mutex_lock(&st->buf_lock);
 	st->tx[0] = ADE7753_READ_REG(reg_address);
-	st->tx[1] = 0;
-	st->tx[2] = 0;
-	st->tx[3] = 0;
 
 	spi_message_init(&msg);
-	spi_message_add_tail(xfers, &msg);
+	spi_message_add_tail(&xfers[0], &msg);
+	spi_message_add_tail(&xfers[1], &msg);
 	ret = spi_sync(st->us, &msg);
 	if (ret) {
 		dev_err(&st->us->dev, "problem when reading 24 bit register 0x%02X",
 				reg_address);
 		goto error_ret;
 	}
-	*val = (st->rx[1] << 16) | (st->rx[2] << 8) | st->rx[3];
+	*val = (st->rx[0] << 16) | (st->rx[1] << 8) | st->rx[2];
 
 error_ret:
 	mutex_unlock(&st->buf_lock);
@@ -186,7 +143,7 @@ static ssize_t ade7753_read_8bit(struct device *dev,
 		char *buf)
 {
 	int ret;
-	u8 val = 0;
+	u8 val;
 	struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
 
 	ret = ade7753_spi_read_reg_8(dev, this_attr->address, &val);
@@ -201,7 +158,7 @@ static ssize_t ade7753_read_16bit(struct device *dev,
 		char *buf)
 {
 	int ret;
-	u16 val = 0;
+	u16 val;
 	struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
 
 	ret = ade7753_spi_read_reg_16(dev, this_attr->address, &val);
@@ -216,14 +173,14 @@ static ssize_t ade7753_read_24bit(struct device *dev,
 		char *buf)
 {
 	int ret;
-	u32 val = 0;
+	u32 val;
 	struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
 
 	ret = ade7753_spi_read_reg_24(dev, this_attr->address, &val);
 	if (ret)
 		return ret;
 
-	return sprintf(buf, "%u\n", val & 0xFFFFFF);
+	return sprintf(buf, "%u\n", val);
 }
 
 static ssize_t ade7753_write_8bit(struct device *dev,
@@ -264,17 +221,12 @@ error_ret:
 
 static int ade7753_reset(struct device *dev)
 {
-	int ret;
 	u16 val;
-	ade7753_spi_read_reg_16(dev,
-			ADE7753_MODE,
-			&val);
+
+	ade7753_spi_read_reg_16(dev, ADE7753_MODE, &val);
 	val |= 1 << 6; /* Software Chip Reset */
-	ret = ade7753_spi_write_reg_16(dev,
-			ADE7753_MODE,
-			val);
 
-	return ret;
+	return ade7753_spi_write_reg_16(dev, ADE7753_MODE, val);
 }
 
 static ssize_t ade7753_write_reset(struct device *dev,
@@ -401,8 +353,6 @@ static int ade7753_set_irq(struct device *dev, bool enable)
 		irqen &= ~(1 << 3);
 
 	ret = ade7753_spi_write_reg_8(dev, ADE7753_IRQEN, irqen);
-	if (ret)
-		goto error_ret;
 
 error_ret:
 	return ret;
@@ -411,17 +361,12 @@ error_ret:
 /* Power down the device */
 int ade7753_stop_device(struct device *dev)
 {
-	int ret;
 	u16 val;
-	ade7753_spi_read_reg_16(dev,
-			ADE7753_MODE,
-			&val);
+
+	ade7753_spi_read_reg_16(dev, ADE7753_MODE, &val);
 	val |= 1 << 4;  /* AD converters can be turned off */
-	ret = ade7753_spi_write_reg_16(dev,
-			ADE7753_MODE,
-			val);
 
-	return ret;
+	return ade7753_spi_write_reg_16(dev, ADE7753_MODE, val);
 }
 
 static int ade7753_initial_setup(struct ade7753_state *st)
@@ -454,16 +399,14 @@ static ssize_t ade7753_read_frequency(struct device *dev,
 	int ret, len = 0;
 	u8 t;
 	int sps;
-	ret = ade7753_spi_read_reg_8(dev,
-			ADE7753_MODE,
-			&t);
+	ret = ade7753_spi_read_reg_8(dev, ADE7753_MODE,	&t);
 	if (ret)
 		return ret;
 
 	t = (t >> 11) & 0x3;
 	sps = 27900 / (1 + t);
 
-	len = sprintf(buf, "%d SPS\n", sps);
+	len = sprintf(buf, "%d\n", sps);
 	return len;
 }
 
@@ -493,24 +436,21 @@ static ssize_t ade7753_write_frequency(struct device *dev,
 	else
 		st->us->max_speed_hz = ADE7753_SPI_FAST;
 
-	ret = ade7753_spi_read_reg_16(dev,
-			ADE7753_MODE,
-			&reg);
+	ret = ade7753_spi_read_reg_16(dev, ADE7753_MODE, &reg);
 	if (ret)
 		goto out;
 
 	reg &= ~(3 << 11);
 	reg |= t << 11;
 
-	ret = ade7753_spi_write_reg_16(dev,
-			ADE7753_MODE,
-			reg);
+	ret = ade7753_spi_write_reg_16(dev, ADE7753_MODE, reg);
 
 out:
 	mutex_unlock(&indio_dev->mlock);
 
 	return ret ? ret : len;
 }
+
 static IIO_DEV_ATTR_TEMP_RAW(ade7753_read_8bit);
 static IIO_CONST_ATTR(temp_offset, "-25 C");
 static IIO_CONST_ATTR(temp_scale, "0.67 C");
@@ -525,14 +465,6 @@ static IIO_CONST_ATTR_SAMP_FREQ_AVAIL("27900 14000 7000 3500");
 
 static IIO_CONST_ATTR(name, "ade7753");
 
-static struct attribute *ade7753_event_attributes[] = {
-	NULL
-};
-
-static struct attribute_group ade7753_event_attribute_group = {
-	.attrs = ade7753_event_attributes,
-};
-
 static struct attribute *ade7753_attributes[] = {
 	&iio_dev_attr_temp_raw.dev_attr.attr,
 	&iio_const_attr_temp_offset.dev_attr.attr,
@@ -607,58 +539,22 @@ static int __devinit ade7753_probe(struct spi_device *spi)
 	}
 
 	st->indio_dev->dev.parent = &spi->dev;
-	st->indio_dev->num_interrupt_lines = 1;
-	st->indio_dev->event_attrs = &ade7753_event_attribute_group;
 	st->indio_dev->attrs = &ade7753_attribute_group;
 	st->indio_dev->dev_data = (void *)(st);
 	st->indio_dev->driver_module = THIS_MODULE;
 	st->indio_dev->modes = INDIO_DIRECT_MODE;
 
-	ret = ade7753_configure_ring(st->indio_dev);
-	if (ret)
-		goto error_free_dev;
-
 	ret = iio_device_register(st->indio_dev);
 	if (ret)
-		goto error_unreg_ring_funcs;
+		goto error_free_dev;
 	regdone = 1;
 
-	ret = ade7753_initialize_ring(st->indio_dev->ring);
-	if (ret) {
-		printk(KERN_ERR "failed to initialize the ring\n");
-		goto error_unreg_ring_funcs;
-	}
-
-	if (spi->irq) {
-		ret = iio_register_interrupt_line(spi->irq,
-				st->indio_dev,
-				0,
-				IRQF_TRIGGER_FALLING,
-				"ade7753");
-		if (ret)
-			goto error_uninitialize_ring;
-
-		ret = ade7753_probe_trigger(st->indio_dev);
-		if (ret)
-			goto error_unregister_line;
-	}
-
 	/* Get the device into a sane initial state */
 	ret = ade7753_initial_setup(st);
 	if (ret)
-		goto error_remove_trigger;
+		goto error_free_dev;
 	return 0;
 
-error_remove_trigger:
-	if (st->indio_dev->modes & INDIO_RING_TRIGGERED)
-		ade7753_remove_trigger(st->indio_dev);
-error_unregister_line:
-	if (st->indio_dev->modes & INDIO_RING_TRIGGERED)
-		iio_unregister_interrupt_line(st->indio_dev, 0);
-error_uninitialize_ring:
-	ade7753_uninitialize_ring(st->indio_dev->ring);
-error_unreg_ring_funcs:
-	ade7753_unconfigure_ring(st->indio_dev);
 error_free_dev:
 	if (regdone)
 		iio_device_unregister(st->indio_dev);
@@ -685,14 +581,6 @@ static int ade7753_remove(struct spi_device *spi)
 	if (ret)
 		goto err_ret;
 
-	flush_scheduled_work();
-
-	ade7753_remove_trigger(indio_dev);
-	if (spi->irq && gpio_is_valid(irq_to_gpio(spi->irq)) > 0)
-		iio_unregister_interrupt_line(indio_dev, 0);
-
-	ade7753_uninitialize_ring(indio_dev->ring);
-	ade7753_unconfigure_ring(indio_dev);
 	iio_device_unregister(indio_dev);
 	kfree(st->tx);
 	kfree(st->rx);
@@ -726,5 +614,5 @@ static __exit void ade7753_exit(void)
 module_exit(ade7753_exit);
 
 MODULE_AUTHOR("Barry Song <21cnbao@gmail.com>");
-MODULE_DESCRIPTION("Analog Devices ADE7753/6 Single-Phase Multifunction Metering IC Driver");
+MODULE_DESCRIPTION("Analog Devices ADE7753/6 Single-Phase Multifunction Meter");
 MODULE_LICENSE("GPL v2");
diff --git a/drivers/staging/iio/meter/ade7753.h b/drivers/staging/iio/meter/ade7753.h
index a3722b8..70dabae 100644
--- a/drivers/staging/iio/meter/ade7753.h
+++ b/drivers/staging/iio/meter/ade7753.h
@@ -60,81 +60,17 @@
 /**
  * struct ade7753_state - device instance specific data
  * @us:			actual spi_device
- * @work_trigger_to_ring: bh for triggered event handling
- * @inter:		used to check if new interrupt has been triggered
- * @last_timestamp:	passing timestamp from th to bh of interrupt handler
  * @indio_dev:		industrial I/O device structure
- * @trig:		data ready trigger registered with iio
  * @tx:			transmit buffer
  * @rx:			recieve buffer
  * @buf_lock:		mutex to protect tx and rx
  **/
 struct ade7753_state {
 	struct spi_device		*us;
-	struct work_struct		work_trigger_to_ring;
-	s64				last_timestamp;
 	struct iio_dev			*indio_dev;
-	struct iio_trigger		*trig;
 	u8				*tx;
 	u8				*rx;
 	struct mutex			buf_lock;
 };
-#if defined(CONFIG_IIO_RING_BUFFER) && defined(THIS_HAS_RING_BUFFER_SUPPORT)
-/* At the moment triggers are only used for ring buffer
- * filling. This may change!
- */
-
-enum ade7753_scan {
-	ADE7753_SCAN_ACTIVE_POWER,
-	ADE7753_SCAN_CH1,
-	ADE7753_SCAN_CH2,
-};
-
-void ade7753_remove_trigger(struct iio_dev *indio_dev);
-int ade7753_probe_trigger(struct iio_dev *indio_dev);
-
-ssize_t ade7753_read_data_from_ring(struct device *dev,
-		struct device_attribute *attr,
-		char *buf);
-
-
-int ade7753_configure_ring(struct iio_dev *indio_dev);
-void ade7753_unconfigure_ring(struct iio_dev *indio_dev);
-
-int ade7753_initialize_ring(struct iio_ring_buffer *ring);
-void ade7753_uninitialize_ring(struct iio_ring_buffer *ring);
-#else /* CONFIG_IIO_RING_BUFFER */
-
-static inline void ade7753_remove_trigger(struct iio_dev *indio_dev)
-{
-}
-static inline int ade7753_probe_trigger(struct iio_dev *indio_dev)
-{
-	return 0;
-}
-
-static inline ssize_t
-ade7753_read_data_from_ring(struct device *dev,
-		struct device_attribute *attr,
-		char *buf)
-{
-	return 0;
-}
-
-static int ade7753_configure_ring(struct iio_dev *indio_dev)
-{
-	return 0;
-}
-static inline void ade7753_unconfigure_ring(struct iio_dev *indio_dev)
-{
-}
-static inline int ade7753_initialize_ring(struct iio_ring_buffer *ring)
-{
-	return 0;
-}
-static inline void ade7753_uninitialize_ring(struct iio_ring_buffer *ring)
-{
-}
-#endif /* CONFIG_IIO_RING_BUFFER */
 
 #endif
-- 
1.7.3.4

[PATCH 2/4] staging:iio:meter remove stubs from ade7754.

[Jonathan Cameron]

General cleanup and use of standard functions to simplfy some spi reads
as well.

Signed-off-by: Jonathan Cameron <jic23@cam.ac.uk>
---
 drivers/staging/iio/meter/ade7754.c |  165 ++++++-----------------------------
 drivers/staging/iio/meter/ade7754.h |   67 --------------
 2 files changed, 26 insertions(+), 206 deletions(-)

diff --git a/drivers/staging/iio/meter/ade7754.c b/drivers/staging/iio/meter/ade7754.c
index 23dedfa..4272818 100644
--- a/drivers/staging/iio/meter/ade7754.c
+++ b/drivers/staging/iio/meter/ade7754.c
@@ -46,25 +46,14 @@ static int ade7754_spi_write_reg_16(struct device *dev,
 		u16 value)
 {
 	int ret;
-	struct spi_message msg;
 	struct iio_dev *indio_dev = dev_get_drvdata(dev);
 	struct ade7754_state *st = iio_dev_get_devdata(indio_dev);
-	struct spi_transfer xfers[] = {
-		{
-			.tx_buf = st->tx,
-			.bits_per_word = 8,
-			.len = 3,
-		}
-	};
 
 	mutex_lock(&st->buf_lock);
 	st->tx[0] = ADE7754_WRITE_REG(reg_address);
 	st->tx[1] = (value >> 8) & 0xFF;
 	st->tx[2] = value & 0xFF;
-
-	spi_message_init(&msg);
-	spi_message_add_tail(xfers, &msg);
-	ret = spi_sync(st->us, &msg);
+	ret = spi_write(st->us, st->tx, 3);
 	mutex_unlock(&st->buf_lock);
 
 	return ret;
@@ -74,73 +63,40 @@ static int ade7754_spi_read_reg_8(struct device *dev,
 		u8 reg_address,
 		u8 *val)
 {
-	struct spi_message msg;
 	struct iio_dev *indio_dev = dev_get_drvdata(dev);
 	struct ade7754_state *st = iio_dev_get_devdata(indio_dev);
 	int ret;
-	struct spi_transfer xfers[] = {
-		{
-			.tx_buf = st->tx,
-			.rx_buf = st->rx,
-			.bits_per_word = 8,
-			.len = 2,
-		},
-	};
-
-	mutex_lock(&st->buf_lock);
-	st->tx[0] = ADE7754_READ_REG(reg_address);
-	st->tx[1] = 0;
 
-	spi_message_init(&msg);
-	spi_message_add_tail(xfers, &msg);
-	ret = spi_sync(st->us, &msg);
-	if (ret) {
+	ret = spi_w8r8(st->us, ADE7754_READ_REG(reg_address));
+	if (ret < 0) {
 		dev_err(&st->us->dev, "problem when reading 8 bit register 0x%02X",
 				reg_address);
-		goto error_ret;
+		return ret;
 	}
-	*val = st->rx[1];
+	*val = ret;
 
-error_ret:
-	mutex_unlock(&st->buf_lock);
-	return ret;
+	return 0;
 }
 
 static int ade7754_spi_read_reg_16(struct device *dev,
 		u8 reg_address,
 		u16 *val)
 {
-	struct spi_message msg;
 	struct iio_dev *indio_dev = dev_get_drvdata(dev);
 	struct ade7754_state *st = iio_dev_get_devdata(indio_dev);
 	int ret;
-	struct spi_transfer xfers[] = {
-		{
-			.tx_buf = st->tx,
-			.rx_buf = st->rx,
-			.bits_per_word = 8,
-			.len = 3,
-		},
-	};
 
-	mutex_lock(&st->buf_lock);
-	st->tx[0] = ADE7754_READ_REG(reg_address);
-	st->tx[1] = 0;
-	st->tx[2] = 0;
-
-	spi_message_init(&msg);
-	spi_message_add_tail(xfers, &msg);
-	ret = spi_sync(st->us, &msg);
-	if (ret) {
+	ret = spi_w8r16(st->us, ADE7754_READ_REG(reg_address));
+	if (ret < 0) {
 		dev_err(&st->us->dev, "problem when reading 16 bit register 0x%02X",
-				reg_address);
-		goto error_ret;
+			reg_address);
+		return ret;
 	}
-	*val = (st->rx[1] << 8) | st->rx[2];
 
-error_ret:
-	mutex_unlock(&st->buf_lock);
-	return ret;
+	*val = ret;
+	*val = be16_to_cpup(val);
+
+	return 0;
 }
 
 static int ade7754_spi_read_reg_24(struct device *dev,
@@ -264,17 +220,11 @@ error_ret:
 
 static int ade7754_reset(struct device *dev)
 {
-	int ret;
 	u8 val;
-	ade7754_spi_read_reg_8(dev,
-			ADE7754_OPMODE,
-			&val);
-	val |= 1 << 6; /* Software Chip Reset */
-	ret = ade7754_spi_write_reg_8(dev,
-			ADE7754_OPMODE,
-			val);
 
-	return ret;
+	ade7754_spi_read_reg_8(dev, ADE7754_OPMODE, &val);
+	val |= 1 << 6; /* Software Chip Reset */
+	return ade7754_spi_write_reg_8(dev, ADE7754_OPMODE, val);
 }
 
 
@@ -431,17 +381,11 @@ error_ret:
 /* Power down the device */
 static int ade7754_stop_device(struct device *dev)
 {
-	int ret;
 	u8 val;
-	ade7754_spi_read_reg_8(dev,
-			ADE7754_OPMODE,
-			&val);
-	val |= 7 << 3;  /* ADE7754 powered down */
-	ret = ade7754_spi_write_reg_8(dev,
-			ADE7754_OPMODE,
-			val);
 
-	return ret;
+	ade7754_spi_read_reg_8(dev, ADE7754_OPMODE, &val);
+	val |= 7 << 3;  /* ADE7754 powered down */
+	return ade7754_spi_write_reg_8(dev, ADE7754_OPMODE, val);
 }
 
 static int ade7754_initial_setup(struct ade7754_state *st)
@@ -471,7 +415,7 @@ static ssize_t ade7754_read_frequency(struct device *dev,
 		struct device_attribute *attr,
 		char *buf)
 {
-	int ret, len = 0;
+	int ret;
 	u8 t;
 	int sps;
 	ret = ade7754_spi_read_reg_8(dev,
@@ -483,8 +427,7 @@ static ssize_t ade7754_read_frequency(struct device *dev,
 	t = (t >> 3) & 0x3;
 	sps = 26000 / (1 + t);
 
-	len = sprintf(buf, "%d SPS\n", sps);
-	return len;
+	return sprintf(buf, "%d\n", sps);
 }
 
 static ssize_t ade7754_write_frequency(struct device *dev,
@@ -513,18 +456,14 @@ static ssize_t ade7754_write_frequency(struct device *dev,
 	else
 		st->us->max_speed_hz = ADE7754_SPI_FAST;
 
-	ret = ade7754_spi_read_reg_8(dev,
-			ADE7754_WAVMODE,
-			&reg);
+	ret = ade7754_spi_read_reg_8(dev, ADE7754_WAVMODE, &reg);
 	if (ret)
 		goto out;
 
 	reg &= ~(3 << 3);
 	reg |= t << 3;
 
-	ret = ade7754_spi_write_reg_8(dev,
-			ADE7754_WAVMODE,
-			reg);
+	ret = ade7754_spi_write_reg_8(dev, ADE7754_WAVMODE, reg);
 
 out:
 	mutex_unlock(&indio_dev->mlock);
@@ -545,14 +484,6 @@ static IIO_CONST_ATTR_SAMP_FREQ_AVAIL("26000 13000 65000 33000");
 
 static IIO_CONST_ATTR(name, "ade7754");
 
-static struct attribute *ade7754_event_attributes[] = {
-	NULL
-};
-
-static struct attribute_group ade7754_event_attribute_group = {
-	.attrs = ade7754_event_attributes,
-};
-
 static struct attribute *ade7754_attributes[] = {
 	&iio_dev_attr_temp_raw.dev_attr.attr,
 	&iio_const_attr_temp_offset.dev_attr.attr,
@@ -633,58 +564,22 @@ static int __devinit ade7754_probe(struct spi_device *spi)
 	}
 
 	st->indio_dev->dev.parent = &spi->dev;
-	st->indio_dev->num_interrupt_lines = 1;
-	st->indio_dev->event_attrs = &ade7754_event_attribute_group;
 	st->indio_dev->attrs = &ade7754_attribute_group;
 	st->indio_dev->dev_data = (void *)(st);
 	st->indio_dev->driver_module = THIS_MODULE;
 	st->indio_dev->modes = INDIO_DIRECT_MODE;
 
-	ret = ade7754_configure_ring(st->indio_dev);
-	if (ret)
-		goto error_free_dev;
-
 	ret = iio_device_register(st->indio_dev);
 	if (ret)
-		goto error_unreg_ring_funcs;
+		goto error_free_dev;
 	regdone = 1;
 
-	ret = ade7754_initialize_ring(st->indio_dev->ring);
-	if (ret) {
-		printk(KERN_ERR "failed to initialize the ring\n");
-		goto error_unreg_ring_funcs;
-	}
-
-	if (spi->irq) {
-		ret = iio_register_interrupt_line(spi->irq,
-				st->indio_dev,
-				0,
-				IRQF_TRIGGER_FALLING,
-				"ade7754");
-		if (ret)
-			goto error_uninitialize_ring;
-
-		ret = ade7754_probe_trigger(st->indio_dev);
-		if (ret)
-			goto error_unregister_line;
-	}
-
 	/* Get the device into a sane initial state */
 	ret = ade7754_initial_setup(st);
 	if (ret)
-		goto error_remove_trigger;
+		goto error_free_dev;
 	return 0;
 
-error_remove_trigger:
-	if (st->indio_dev->modes & INDIO_RING_TRIGGERED)
-		ade7754_remove_trigger(st->indio_dev);
-error_unregister_line:
-	if (st->indio_dev->modes & INDIO_RING_TRIGGERED)
-		iio_unregister_interrupt_line(st->indio_dev, 0);
-error_uninitialize_ring:
-	ade7754_uninitialize_ring(st->indio_dev->ring);
-error_unreg_ring_funcs:
-	ade7754_unconfigure_ring(st->indio_dev);
 error_free_dev:
 	if (regdone)
 		iio_device_unregister(st->indio_dev);
@@ -711,14 +606,6 @@ static int ade7754_remove(struct spi_device *spi)
 	if (ret)
 		goto err_ret;
 
-	flush_scheduled_work();
-
-	ade7754_remove_trigger(indio_dev);
-	if (spi->irq)
-		iio_unregister_interrupt_line(indio_dev, 0);
-
-	ade7754_uninitialize_ring(indio_dev->ring);
-	ade7754_unconfigure_ring(indio_dev);
 	iio_device_unregister(indio_dev);
 	kfree(st->tx);
 	kfree(st->rx);
diff --git a/drivers/staging/iio/meter/ade7754.h b/drivers/staging/iio/meter/ade7754.h
index f6a3e4b..8faa9b3 100644
--- a/drivers/staging/iio/meter/ade7754.h
+++ b/drivers/staging/iio/meter/ade7754.h
@@ -78,84 +78,17 @@
 /**
  * struct ade7754_state - device instance specific data
  * @us:			actual spi_device
- * @work_trigger_to_ring: bh for triggered event handling
- * @inter:		used to check if new interrupt has been triggered
- * @last_timestamp:	passing timestamp from th to bh of interrupt handler
  * @indio_dev:		industrial I/O device structure
- * @trig:		data ready trigger registered with iio
  * @tx:			transmit buffer
  * @rx:			recieve buffer
  * @buf_lock:		mutex to protect tx and rx
  **/
 struct ade7754_state {
 	struct spi_device		*us;
-	struct work_struct		work_trigger_to_ring;
-	s64				last_timestamp;
 	struct iio_dev			*indio_dev;
-	struct iio_trigger		*trig;
 	u8				*tx;
 	u8				*rx;
 	struct mutex			buf_lock;
 };
-#if defined(CONFIG_IIO_RING_BUFFER) && defined(THIS_HAS_RING_BUFFER_SUPPORT)
-/* At the moment triggers are only used for ring buffer
- * filling. This may change!
- */
-
-enum ade7754_scan {
-	ADE7754_SCAN_PHA_V,
-	ADE7754_SCAN_PHB_V,
-	ADE7754_SCAN_PHC_V,
-	ADE7754_SCAN_PHA_I,
-	ADE7754_SCAN_PHB_I,
-	ADE7754_SCAN_PHC_I,
-};
-
-void ade7754_remove_trigger(struct iio_dev *indio_dev);
-int ade7754_probe_trigger(struct iio_dev *indio_dev);
-
-ssize_t ade7754_read_data_from_ring(struct device *dev,
-				      struct device_attribute *attr,
-				      char *buf);
-
-
-int ade7754_configure_ring(struct iio_dev *indio_dev);
-void ade7754_unconfigure_ring(struct iio_dev *indio_dev);
-
-int ade7754_initialize_ring(struct iio_ring_buffer *ring);
-void ade7754_uninitialize_ring(struct iio_ring_buffer *ring);
-#else /* CONFIG_IIO_RING_BUFFER */
-
-static inline void ade7754_remove_trigger(struct iio_dev *indio_dev)
-{
-}
-static inline int ade7754_probe_trigger(struct iio_dev *indio_dev)
-{
-	return 0;
-}
-
-static inline ssize_t
-ade7754_read_data_from_ring(struct device *dev,
-			      struct device_attribute *attr,
-			      char *buf)
-{
-	return 0;
-}
-
-static int ade7754_configure_ring(struct iio_dev *indio_dev)
-{
-	return 0;
-}
-static inline void ade7754_unconfigure_ring(struct iio_dev *indio_dev)
-{
-}
-static inline int ade7754_initialize_ring(struct iio_ring_buffer *ring)
-{
-	return 0;
-}
-static inline void ade7754_uninitialize_ring(struct iio_ring_buffer *ring)
-{
-}
-#endif /* CONFIG_IIO_RING_BUFFER */
 
 #endif
-- 
1.7.3.4

[PATCH 3/4] staging:iio:meter remove stubs from ade7759.

[Jonathan Cameron]

General cleanup and use of standard functions to simplfy some spi reads
as well.

Signed-off-by: Jonathan Cameron <jic23@cam.ac.uk>
---
 drivers/staging/iio/meter/ade7759.c |  163 ++++++----------------------------
 drivers/staging/iio/meter/ade7759.h |   65 --------------
 2 files changed, 29 insertions(+), 199 deletions(-)

diff --git a/drivers/staging/iio/meter/ade7759.c b/drivers/staging/iio/meter/ade7759.c
index fafc3c1..ec5fd6b 100644
--- a/drivers/staging/iio/meter/ade7759.c
+++ b/drivers/staging/iio/meter/ade7759.c
@@ -46,25 +46,14 @@ static int ade7759_spi_write_reg_16(struct device *dev,
 		u16 value)
 {
 	int ret;
-	struct spi_message msg;
 	struct iio_dev *indio_dev = dev_get_drvdata(dev);
 	struct ade7759_state *st = iio_dev_get_devdata(indio_dev);
-	struct spi_transfer xfers[] = {
-		{
-			.tx_buf = st->tx,
-			.bits_per_word = 8,
-			.len = 3,
-		}
-	};
-
+	
 	mutex_lock(&st->buf_lock);
 	st->tx[0] = ADE7759_WRITE_REG(reg_address);
 	st->tx[1] = (value >> 8) & 0xFF;
 	st->tx[2] = value & 0xFF;
-
-	spi_message_init(&msg);
-	spi_message_add_tail(xfers, &msg);
-	ret = spi_sync(st->us, &msg);
+	ret = spi_write(st->us, st->tx, 3);
 	mutex_unlock(&st->buf_lock);
 
 	return ret;
@@ -74,73 +63,40 @@ static int ade7759_spi_read_reg_8(struct device *dev,
 		u8 reg_address,
 		u8 *val)
 {
-	struct spi_message msg;
 	struct iio_dev *indio_dev = dev_get_drvdata(dev);
 	struct ade7759_state *st = iio_dev_get_devdata(indio_dev);
 	int ret;
-	struct spi_transfer xfers[] = {
-		{
-			.tx_buf = st->tx,
-			.rx_buf = st->rx,
-			.bits_per_word = 8,
-			.len = 2,
-		},
-	};
-
-	mutex_lock(&st->buf_lock);
-	st->tx[0] = ADE7759_READ_REG(reg_address);
-	st->tx[1] = 0;
 
-	spi_message_init(&msg);
-	spi_message_add_tail(xfers, &msg);
-	ret = spi_sync(st->us, &msg);
-	if (ret) {
+	ret = spi_w8r8(st->us, ADE7759_READ_REG(reg_address));
+	if (ret < 0) {
 		dev_err(&st->us->dev, "problem when reading 8 bit register 0x%02X",
 				reg_address);
-		goto error_ret;
+		return ret;
 	}
-	*val = st->rx[1];
+	*val = ret;
 
-error_ret:
-	mutex_unlock(&st->buf_lock);
-	return ret;
+	return 0;
 }
 
 static int ade7759_spi_read_reg_16(struct device *dev,
 		u8 reg_address,
 		u16 *val)
 {
-	struct spi_message msg;
 	struct iio_dev *indio_dev = dev_get_drvdata(dev);
 	struct ade7759_state *st = iio_dev_get_devdata(indio_dev);
 	int ret;
-	struct spi_transfer xfers[] = {
-		{
-			.tx_buf = st->tx,
-			.rx_buf = st->rx,
-			.bits_per_word = 8,
-			.len = 3,
-		},
-	};
 
-	mutex_lock(&st->buf_lock);
-	st->tx[0] = ADE7759_READ_REG(reg_address);
-	st->tx[1] = 0;
-	st->tx[2] = 0;
-
-	spi_message_init(&msg);
-	spi_message_add_tail(xfers, &msg);
-	ret = spi_sync(st->us, &msg);
-	if (ret) {
+	ret = spi_w8r16(st->us, ADE7759_READ_REG(reg_address));
+	if (ret < 0) {
 		dev_err(&st->us->dev, "problem when reading 16 bit register 0x%02X",
-				reg_address);
-		goto error_ret;
+			reg_address);
+		return ret;
 	}
-	*val = (st->rx[1] << 8) | st->rx[2];
 
-error_ret:
-	mutex_unlock(&st->buf_lock);
-	return ret;
+	*val = ret;
+	*val = be16_to_cpup(val);
+
+	return 0;
 }
 
 static int ade7759_spi_read_reg_40(struct device *dev,
@@ -354,8 +310,6 @@ static int ade7759_set_irq(struct device *dev, bool enable)
 		irqen &= ~(1 << 3);
 
 	ret = ade7759_spi_write_reg_8(dev, ADE7759_IRQEN, irqen);
-	if (ret)
-		goto error_ret;
 
 error_ret:
 	return ret;
@@ -364,17 +318,14 @@ error_ret:
 /* Power down the device */
 int ade7759_stop_device(struct device *dev)
 {
-	int ret;
 	u16 val;
+
 	ade7759_spi_read_reg_16(dev,
 			ADE7759_MODE,
 			&val);
 	val |= 1 << 4;  /* AD converters can be turned off */
-	ret = ade7759_spi_write_reg_16(dev,
-			ADE7759_MODE,
-			val);
 
-	return ret;
+	return ade7759_spi_write_reg_16(dev, ADE7759_MODE, val);
 }
 
 static int ade7759_initial_setup(struct ade7759_state *st)
@@ -404,7 +355,7 @@ static ssize_t ade7759_read_frequency(struct device *dev,
 		struct device_attribute *attr,
 		char *buf)
 {
-	int ret, len = 0;
+	int ret;
 	u16 t;
 	int sps;
 	ret = ade7759_spi_read_reg_16(dev,
@@ -416,8 +367,7 @@ static ssize_t ade7759_read_frequency(struct device *dev,
 	t = (t >> 3) & 0x3;
 	sps = 27900 / (1 + t);
 
-	len = sprintf(buf, "%d SPS\n", sps);
-	return len;
+	return sprintf(buf, "%d\n", sps);
 }
 
 static ssize_t ade7759_write_frequency(struct device *dev,
@@ -446,18 +396,14 @@ static ssize_t ade7759_write_frequency(struct device *dev,
 	else
 		st->us->max_speed_hz = ADE7759_SPI_FAST;
 
-	ret = ade7759_spi_read_reg_16(dev,
-			ADE7759_MODE,
-			&reg);
+	ret = ade7759_spi_read_reg_16(dev, ADE7759_MODE, &reg);
 	if (ret)
 		goto out;
 
 	reg &= ~(3 << 13);
 	reg |= t << 13;
 
-	ret = ade7759_spi_write_reg_16(dev,
-			ADE7759_MODE,
-			reg);
+	ret = ade7759_spi_write_reg_16(dev, ADE7759_MODE, reg);
 
 out:
 	mutex_unlock(&indio_dev->mlock);
@@ -478,14 +424,6 @@ static IIO_CONST_ATTR_SAMP_FREQ_AVAIL("27900 14000 7000 3500");
 
 static IIO_CONST_ATTR(name, "ade7759");
 
-static struct attribute *ade7759_event_attributes[] = {
-	NULL
-};
-
-static struct attribute_group ade7759_event_attribute_group = {
-	.attrs = ade7759_event_attributes,
-};
-
 static struct attribute *ade7759_attributes[] = {
 	&iio_dev_attr_temp_raw.dev_attr.attr,
 	&iio_const_attr_temp_offset.dev_attr.attr,
@@ -517,7 +455,7 @@ static const struct attribute_group ade7759_attribute_group = {
 
 static int __devinit ade7759_probe(struct spi_device *spi)
 {
-	int ret, regdone = 0;
+	int ret;
 	struct ade7759_state *st = kzalloc(sizeof *st, GFP_KERNEL);
 	if (!st) {
 		ret =  -ENOMEM;
@@ -548,62 +486,27 @@ static int __devinit ade7759_probe(struct spi_device *spi)
 
 	st->indio_dev->dev.parent = &spi->dev;
 	st->indio_dev->num_interrupt_lines = 1;
-	st->indio_dev->event_attrs = &ade7759_event_attribute_group;
+
 	st->indio_dev->attrs = &ade7759_attribute_group;
 	st->indio_dev->dev_data = (void *)(st);
 	st->indio_dev->driver_module = THIS_MODULE;
 	st->indio_dev->modes = INDIO_DIRECT_MODE;
 
-	ret = ade7759_configure_ring(st->indio_dev);
-	if (ret)
-		goto error_free_dev;
-
 	ret = iio_device_register(st->indio_dev);
 	if (ret)
-		goto error_unreg_ring_funcs;
-	regdone = 1;
-
-	ret = ade7759_initialize_ring(st->indio_dev->ring);
-	if (ret) {
-		printk(KERN_ERR "failed to initialize the ring\n");
-		goto error_unreg_ring_funcs;
-	}
-
-	if (spi->irq) {
-		ret = iio_register_interrupt_line(spi->irq,
-				st->indio_dev,
-				0,
-				IRQF_TRIGGER_FALLING,
-				"ade7759");
-		if (ret)
-			goto error_uninitialize_ring;
-
-		ret = ade7759_probe_trigger(st->indio_dev);
-		if (ret)
-			goto error_unregister_line;
-	}
+		goto error_free_dev;
 
 	/* Get the device into a sane initial state */
 	ret = ade7759_initial_setup(st);
 	if (ret)
-		goto error_remove_trigger;
+		goto error_unreg_dev;
 	return 0;
 
-error_remove_trigger:
-	if (st->indio_dev->modes & INDIO_RING_TRIGGERED)
-		ade7759_remove_trigger(st->indio_dev);
-error_unregister_line:
-	if (st->indio_dev->modes & INDIO_RING_TRIGGERED)
-		iio_unregister_interrupt_line(st->indio_dev, 0);
-error_uninitialize_ring:
-	ade7759_uninitialize_ring(st->indio_dev->ring);
-error_unreg_ring_funcs:
-	ade7759_unconfigure_ring(st->indio_dev);
+
+error_unreg_dev:
+	iio_device_unregister(st->indio_dev);
 error_free_dev:
-	if (regdone)
-		iio_device_unregister(st->indio_dev);
-	else
-		iio_free_device(st->indio_dev);
+	iio_free_device(st->indio_dev);
 error_free_tx:
 	kfree(st->tx);
 error_free_rx:
@@ -625,14 +528,6 @@ static int ade7759_remove(struct spi_device *spi)
 	if (ret)
 		goto err_ret;
 
-	flush_scheduled_work();
-
-	ade7759_remove_trigger(indio_dev);
-	if (spi->irq && gpio_is_valid(irq_to_gpio(spi->irq)) > 0)
-		iio_unregister_interrupt_line(indio_dev, 0);
-
-	ade7759_uninitialize_ring(indio_dev->ring);
-	ade7759_unconfigure_ring(indio_dev);
 	iio_device_unregister(indio_dev);
 	kfree(st->tx);
 	kfree(st->rx);
diff --git a/drivers/staging/iio/meter/ade7759.h b/drivers/staging/iio/meter/ade7759.h
index 813dea2..e9d1c43 100644
--- a/drivers/staging/iio/meter/ade7759.h
+++ b/drivers/staging/iio/meter/ade7759.h
@@ -41,82 +41,17 @@
 /**
  * struct ade7759_state - device instance specific data
  * @us:			actual spi_device
- * @work_trigger_to_ring: bh for triggered event handling
- * @inter:		used to check if new interrupt has been triggered
- * @last_timestamp:	passing timestamp from th to bh of interrupt handler
  * @indio_dev:		industrial I/O device structure
- * @trig:		data ready trigger registered with iio
  * @tx:			transmit buffer
  * @rx:			recieve buffer
  * @buf_lock:		mutex to protect tx and rx
  **/
 struct ade7759_state {
 	struct spi_device		*us;
-	struct work_struct		work_trigger_to_ring;
-	s64				last_timestamp;
 	struct iio_dev			*indio_dev;
-	struct iio_trigger		*trig;
 	u8				*tx;
 	u8				*rx;
 	struct mutex			buf_lock;
 };
-#if defined(CONFIG_IIO_RING_BUFFER) && defined(THIS_HAS_RING_BUFFER_SUPPORT)
-/* At the moment triggers are only used for ring buffer
- * filling. This may change!
- */
-
-enum ade7759_scan {
-	ADE7759_SCAN_ACTIVE_POWER,
-	ADE7759_SCAN_CH1_CH2,
-	ADE7759_SCAN_CH1,
-	ADE7759_SCAN_CH2,
-};
-
-void ade7759_remove_trigger(struct iio_dev *indio_dev);
-int ade7759_probe_trigger(struct iio_dev *indio_dev);
-
-ssize_t ade7759_read_data_from_ring(struct device *dev,
-		struct device_attribute *attr,
-		char *buf);
-
-
-int ade7759_configure_ring(struct iio_dev *indio_dev);
-void ade7759_unconfigure_ring(struct iio_dev *indio_dev);
-
-int ade7759_initialize_ring(struct iio_ring_buffer *ring);
-void ade7759_uninitialize_ring(struct iio_ring_buffer *ring);
-#else /* CONFIG_IIO_RING_BUFFER */
-
-static inline void ade7759_remove_trigger(struct iio_dev *indio_dev)
-{
-}
-static inline int ade7759_probe_trigger(struct iio_dev *indio_dev)
-{
-	return 0;
-}
-
-static inline ssize_t
-ade7759_read_data_from_ring(struct device *dev,
-		struct device_attribute *attr,
-		char *buf)
-{
-	return 0;
-}
-
-static int ade7759_configure_ring(struct iio_dev *indio_dev)
-{
-	return 0;
-}
-static inline void ade7759_unconfigure_ring(struct iio_dev *indio_dev)
-{
-}
-static inline int ade7759_initialize_ring(struct iio_ring_buffer *ring)
-{
-	return 0;
-}
-static inline void ade7759_uninitialize_ring(struct iio_ring_buffer *ring)
-{
-}
-#endif /* CONFIG_IIO_RING_BUFFER */
 
 #endif
-- 
1.7.3.4

[PATCH 4/4] staging:iio:meter remove stubs from ade7854.

[Jonathan Cameron]

General cleanup and use of standard functions to simplfy some spi reads
as well.

Also added a device id table to the spi version.

Signed-off-by: Jonathan Cameron <jic23@cam.ac.uk>
---
 drivers/staging/iio/meter/ade7854-spi.c |  120 +++++++++++++++++--------------
 drivers/staging/iio/meter/ade7854.c     |   78 +++-----------------
 drivers/staging/iio/meter/ade7854.h     |   69 ------------------
 3 files changed, 76 insertions(+), 191 deletions(-)

diff --git a/drivers/staging/iio/meter/ade7854-spi.c b/drivers/staging/iio/meter/ade7854-spi.c
index fe58103e..84da8fb 100644
--- a/drivers/staging/iio/meter/ade7854-spi.c
+++ b/drivers/staging/iio/meter/ade7854-spi.c
@@ -22,12 +22,10 @@ static int ade7854_spi_write_reg_8(struct device *dev,
 	struct spi_message msg;
 	struct iio_dev *indio_dev = dev_get_drvdata(dev);
 	struct ade7854_state *st = iio_dev_get_devdata(indio_dev);
-	struct spi_transfer xfers[] = {
-		{
-			.tx_buf = st->tx,
-			.bits_per_word = 8,
-			.len = 4,
-		}
+	struct spi_transfer xfer = {
+		.tx_buf = st->tx,
+		.bits_per_word = 8,
+		.len = 4,
 	};
 
 	mutex_lock(&st->buf_lock);
@@ -37,7 +35,7 @@ static int ade7854_spi_write_reg_8(struct device *dev,
 	st->tx[3] = value & 0xFF;
 
 	spi_message_init(&msg);
-	spi_message_add_tail(xfers, &msg);
+	spi_message_add_tail(&xfer, &msg);
 	ret = spi_sync(st->spi, &msg);
 	mutex_unlock(&st->buf_lock);
 
@@ -52,12 +50,10 @@ static int ade7854_spi_write_reg_16(struct device *dev,
 	struct spi_message msg;
 	struct iio_dev *indio_dev = dev_get_drvdata(dev);
 	struct ade7854_state *st = iio_dev_get_devdata(indio_dev);
-	struct spi_transfer xfers[] = {
-		{
-			.tx_buf = st->tx,
-			.bits_per_word = 8,
-			.len = 5,
-		}
+	struct spi_transfer xfer = {
+		.tx_buf = st->tx,
+		.bits_per_word = 8,
+		.len = 5,
 	};
 
 	mutex_lock(&st->buf_lock);
@@ -68,7 +64,7 @@ static int ade7854_spi_write_reg_16(struct device *dev,
 	st->tx[4] = value & 0xFF;
 
 	spi_message_init(&msg);
-	spi_message_add_tail(xfers, &msg);
+	spi_message_add_tail(&xfer, &msg);
 	ret = spi_sync(st->spi, &msg);
 	mutex_unlock(&st->buf_lock);
 
@@ -83,12 +79,10 @@ static int ade7854_spi_write_reg_24(struct device *dev,
 	struct spi_message msg;
 	struct iio_dev *indio_dev = dev_get_drvdata(dev);
 	struct ade7854_state *st = iio_dev_get_devdata(indio_dev);
-	struct spi_transfer xfers[] = {
-		{
-			.tx_buf = st->tx,
-			.bits_per_word = 8,
-			.len = 6,
-		}
+	struct spi_transfer xfer = {
+		.tx_buf = st->tx,
+		.bits_per_word = 8,
+		.len = 6,
 	};
 
 	mutex_lock(&st->buf_lock);
@@ -100,7 +94,7 @@ static int ade7854_spi_write_reg_24(struct device *dev,
 	st->tx[5] = value & 0xFF;
 
 	spi_message_init(&msg);
-	spi_message_add_tail(xfers, &msg);
+	spi_message_add_tail(&xfer, &msg);
 	ret = spi_sync(st->spi, &msg);
 	mutex_unlock(&st->buf_lock);
 
@@ -115,12 +109,10 @@ static int ade7854_spi_write_reg_32(struct device *dev,
 	struct spi_message msg;
 	struct iio_dev *indio_dev = dev_get_drvdata(dev);
 	struct ade7854_state *st = iio_dev_get_devdata(indio_dev);
-	struct spi_transfer xfers[] = {
-		{
-			.tx_buf = st->tx,
-			.bits_per_word = 8,
-			.len = 7,
-		}
+	struct spi_transfer xfer = {
+		.tx_buf = st->tx,
+		.bits_per_word = 8,
+		.len = 7,
 	};
 
 	mutex_lock(&st->buf_lock);
@@ -133,7 +125,7 @@ static int ade7854_spi_write_reg_32(struct device *dev,
 	st->tx[6] = value & 0xFF;
 
 	spi_message_init(&msg);
-	spi_message_add_tail(xfers, &msg);
+	spi_message_add_tail(&xfer, &msg);
 	ret = spi_sync(st->spi, &msg);
 	mutex_unlock(&st->buf_lock);
 
@@ -152,8 +144,12 @@ static int ade7854_spi_read_reg_8(struct device *dev,
 		{
 			.tx_buf = st->tx,
 			.bits_per_word = 8,
-			.len = 4,
-		},
+			.len = 3,
+		}, {
+			.rx_buf = st->rx,
+			.bits_per_word = 8,
+			.len = 1,
+		}
 	};
 
 	mutex_lock(&st->buf_lock);
@@ -161,17 +157,17 @@ static int ade7854_spi_read_reg_8(struct device *dev,
 	st->tx[0] = ADE7854_READ_REG;
 	st->tx[1] = (reg_address >> 8) & 0xFF;
 	st->tx[2] = reg_address & 0xFF;
-	st->tx[3] = 0;
 
 	spi_message_init(&msg);
-	spi_message_add_tail(xfers, &msg);
+	spi_message_add_tail(&xfers[0], &msg);
+	spi_message_add_tail(&xfers[1], &msg);
 	ret = spi_sync(st->spi, &msg);
 	if (ret) {
 		dev_err(&st->spi->dev, "problem when reading 8 bit register 0x%02X",
 				reg_address);
 		goto error_ret;
 	}
-	*val = st->rx[3];
+	*val = st->rx[0];
 
 error_ret:
 	mutex_unlock(&st->buf_lock);
@@ -190,26 +186,29 @@ static int ade7854_spi_read_reg_16(struct device *dev,
 		{
 			.tx_buf = st->tx,
 			.bits_per_word = 8,
-			.len = 5,
-		},
+			.len = 3,
+		}, {
+			.rx_buf = st->rx,
+			.bits_per_word = 8,
+			.len = 2,
+		}
 	};
 
 	mutex_lock(&st->buf_lock);
 	st->tx[0] = ADE7854_READ_REG;
 	st->tx[1] = (reg_address >> 8) & 0xFF;
 	st->tx[2] = reg_address & 0xFF;
-	st->tx[3] = 0;
-	st->tx[4] = 0;
 
 	spi_message_init(&msg);
-	spi_message_add_tail(xfers, &msg);
+	spi_message_add_tail(&xfers[0], &msg);
+	spi_message_add_tail(&xfers[1], &msg);
 	ret = spi_sync(st->spi, &msg);
 	if (ret) {
 		dev_err(&st->spi->dev, "problem when reading 16 bit register 0x%02X",
 				reg_address);
 		goto error_ret;
 	}
-	*val = (st->rx[3] << 8) | st->rx[4];
+	*val = be16_to_cpup((const __be16 *)st->rx);
 
 error_ret:
 	mutex_unlock(&st->buf_lock);
@@ -228,8 +227,12 @@ static int ade7854_spi_read_reg_24(struct device *dev,
 		{
 			.tx_buf = st->tx,
 			.bits_per_word = 8,
-			.len = 6,
-		},
+			.len = 3,
+		}, {
+			.rx_buf = st->rx,
+			.bits_per_word = 8,
+			.len = 3,
+		}
 	};
 
 	mutex_lock(&st->buf_lock);
@@ -237,19 +240,17 @@ static int ade7854_spi_read_reg_24(struct device *dev,
 	st->tx[0] = ADE7854_READ_REG;
 	st->tx[1] = (reg_address >> 8) & 0xFF;
 	st->tx[2] = reg_address & 0xFF;
-	st->tx[3] = 0;
-	st->tx[4] = 0;
-	st->tx[5] = 0;
 
 	spi_message_init(&msg);
-	spi_message_add_tail(xfers, &msg);
+	spi_message_add_tail(&xfers[0], &msg);
+	spi_message_add_tail(&xfers[1], &msg);
 	ret = spi_sync(st->spi, &msg);
 	if (ret) {
 		dev_err(&st->spi->dev, "problem when reading 24 bit register 0x%02X",
 				reg_address);
 		goto error_ret;
 	}
-	*val = (st->rx[3] << 16) | (st->rx[4] << 8) | st->rx[5];
+	*val = (st->rx[0] << 16) | (st->rx[1] << 8) | st->rx[2];
 
 error_ret:
 	mutex_unlock(&st->buf_lock);
@@ -268,8 +269,12 @@ static int ade7854_spi_read_reg_32(struct device *dev,
 		{
 			.tx_buf = st->tx,
 			.bits_per_word = 8,
-			.len = 7,
-		},
+			.len = 3,
+		}, {
+			.rx_buf = st->rx,
+			.bits_per_word = 8,
+			.len = 4,
+		}
 	};
 
 	mutex_lock(&st->buf_lock);
@@ -277,20 +282,17 @@ static int ade7854_spi_read_reg_32(struct device *dev,
 	st->tx[0] = ADE7854_READ_REG;
 	st->tx[1] = (reg_address >> 8) & 0xFF;
 	st->tx[2] = reg_address & 0xFF;
-	st->tx[3] = 0;
-	st->tx[4] = 0;
-	st->tx[5] = 0;
-	st->tx[6] = 0;
 
 	spi_message_init(&msg);
-	spi_message_add_tail(xfers, &msg);
+	spi_message_add_tail(&xfers[0], &msg);
+	spi_message_add_tail(&xfers[1], &msg);
 	ret = spi_sync(st->spi, &msg);
 	if (ret) {
 		dev_err(&st->spi->dev, "problem when reading 32 bit register 0x%02X",
 				reg_address);
 		goto error_ret;
 	}
-	*val = (st->rx[3] << 24) | (st->rx[4] << 16) | (st->rx[5] << 8) | st->rx[6];
+	*val = be32_to_cpup((const __be32 *)st->rx);
 
 error_ret:
 	mutex_unlock(&st->buf_lock);
@@ -333,6 +335,13 @@ static int ade7854_spi_remove(struct spi_device *spi)
 
 	return 0;
 }
+static const struct spi_device_id ade7854_id[] = {
+	{ "ade7854", 0 },
+	{ "ade7858", 0 },
+	{ "ade7868", 0 },
+	{ "ade7878", 0 },
+	{ }
+};
 
 static struct spi_driver ade7854_driver = {
 	.driver = {
@@ -341,6 +350,7 @@ static struct spi_driver ade7854_driver = {
 	},
 	.probe = ade7854_spi_probe,
 	.remove = __devexit_p(ade7854_spi_remove),
+	.id_table = ade7854_id,
 };
 
 static __init int ade7854_init(void)
@@ -356,5 +366,5 @@ static __exit void ade7854_exit(void)
 module_exit(ade7854_exit);
 
 MODULE_AUTHOR("Barry Song <21cnbao@gmail.com>");
-MODULE_DESCRIPTION("Analog Devices ADE7854/58/68/78 Polyphase Multifunction Energy Metering IC SPI Driver");
+MODULE_DESCRIPTION("Analog Devices ADE7854/58/68/78 SPI Driver");
 MODULE_LICENSE("GPL v2");
diff --git a/drivers/staging/iio/meter/ade7854.c b/drivers/staging/iio/meter/ade7854.c
index a13d504..866e585 100644
--- a/drivers/staging/iio/meter/ade7854.c
+++ b/drivers/staging/iio/meter/ade7854.c
@@ -61,7 +61,7 @@ static ssize_t ade7854_read_24bit(struct device *dev,
 		char *buf)
 {
 	int ret;
-	u32 val = 0;
+	u32 val;
 	struct iio_dev *indio_dev = dev_get_drvdata(dev);
 	struct ade7854_state *st = iio_dev_get_devdata(indio_dev);
 	struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
@@ -70,7 +70,7 @@ static ssize_t ade7854_read_24bit(struct device *dev,
 	if (ret)
 		return ret;
 
-	return sprintf(buf, "%u\n", val & 0xFFFFFF);
+	return sprintf(buf, "%u\n", val);
 }
 
 static ssize_t ade7854_read_32bit(struct device *dev,
@@ -178,15 +178,12 @@ static int ade7854_reset(struct device *dev)
 {
 	struct iio_dev *indio_dev = dev_get_drvdata(dev);
 	struct ade7854_state *st = iio_dev_get_devdata(indio_dev);
-
-	int ret;
 	u16 val;
 
 	st->read_reg_16(dev, ADE7854_CONFIG, &val);
 	val |= 1 << 7; /* Software Chip Reset */
-	ret = st->write_reg_16(dev, ADE7854_CONFIG, val);
 
-	return ret;
+	return st->write_reg_16(dev, ADE7854_CONFIG, val);
 }
 
 
@@ -477,14 +474,6 @@ static IIO_CONST_ATTR_SAMP_FREQ_AVAIL("8000");
 
 static IIO_CONST_ATTR(name, "ade7854");
 
-static struct attribute *ade7854_event_attributes[] = {
-	NULL
-};
-
-static struct attribute_group ade7854_event_attribute_group = {
-	.attrs = ade7854_event_attributes,
-};
-
 static struct attribute *ade7854_attributes[] = {
 	&iio_dev_attr_aigain.dev_attr.attr,
 	&iio_dev_attr_bigain.dev_attr.attr,
@@ -564,7 +553,7 @@ static const struct attribute_group ade7854_attribute_group = {
 
 int ade7854_probe(struct ade7854_state *st, struct device *dev)
 {
-	int ret, regdone = 0;
+	int ret;
 
 	/* Allocate the comms buffers */
 	st->rx = kzalloc(sizeof(*st->rx)*ADE7854_MAX_RX, GFP_KERNEL);
@@ -586,71 +575,34 @@ int ade7854_probe(struct ade7854_state *st, struct device *dev)
 	}
 
 	st->indio_dev->dev.parent = dev;
-	st->indio_dev->num_interrupt_lines = 1;
-	st->indio_dev->event_attrs = &ade7854_event_attribute_group;
 	st->indio_dev->attrs = &ade7854_attribute_group;
 	st->indio_dev->dev_data = (void *)(st);
 	st->indio_dev->driver_module = THIS_MODULE;
 	st->indio_dev->modes = INDIO_DIRECT_MODE;
 
-	ret = ade7854_configure_ring(st->indio_dev);
-	if (ret)
-		goto error_free_dev;
-
 	ret = iio_device_register(st->indio_dev);
 	if (ret)
-		goto error_unreg_ring_funcs;
-	regdone = 1;
-
-	ret = ade7854_initialize_ring(st->indio_dev->ring);
-	if (ret) {
-		printk(KERN_ERR "failed to initialize the ring\n");
-		goto error_unreg_ring_funcs;
-	}
+		goto error_free_dev;
 
-	if (st->irq) {
-		ret = iio_register_interrupt_line(st->irq,
-				st->indio_dev,
-				0,
-				IRQF_TRIGGER_FALLING,
-				"ade7854");
-		if (ret)
-			goto error_uninitialize_ring;
-
-		ret = ade7854_probe_trigger(st->indio_dev);
-		if (ret)
-			goto error_unregister_line;
-	}
 	/* Get the device into a sane initial state */
 	ret = ade7854_initial_setup(st);
 	if (ret)
-		goto error_remove_trigger;
+		goto error_unreg_dev;
 
 	return 0;
 
-error_remove_trigger:
-	if (st->indio_dev->modes & INDIO_RING_TRIGGERED)
-		ade7854_remove_trigger(st->indio_dev);
-error_unregister_line:
-	if (st->indio_dev->modes & INDIO_RING_TRIGGERED)
-		iio_unregister_interrupt_line(st->indio_dev, 0);
-error_uninitialize_ring:
-	ade7854_uninitialize_ring(st->indio_dev->ring);
-error_unreg_ring_funcs:
-	ade7854_unconfigure_ring(st->indio_dev);
+error_unreg_dev:
+	iio_device_unregister(st->indio_dev);
 error_free_dev:
-	if (regdone)
-		iio_device_unregister(st->indio_dev);
-	else
-		iio_free_device(st->indio_dev);
+	iio_free_device(st->indio_dev);
 error_free_tx:
 	kfree(st->tx);
 error_free_rx:
 	kfree(st->rx);
 error_free_st:
 	kfree(st);
-	return ret;
 
+	return ret;
 }
 EXPORT_SYMBOL(ade7854_probe);
 
@@ -658,14 +610,6 @@ int ade7854_remove(struct ade7854_state *st)
 {
 	struct iio_dev *indio_dev = st->indio_dev;
 
-	flush_scheduled_work();
-
-	ade7854_remove_trigger(indio_dev);
-	if (st->irq)
-		iio_unregister_interrupt_line(indio_dev, 0);
-
-	ade7854_uninitialize_ring(indio_dev->ring);
-	ade7854_unconfigure_ring(indio_dev);
 	iio_device_unregister(indio_dev);
 	kfree(st->tx);
 	kfree(st->rx);
@@ -676,5 +620,5 @@ int ade7854_remove(struct ade7854_state *st)
 EXPORT_SYMBOL(ade7854_remove);
 
 MODULE_AUTHOR("Barry Song <21cnbao@gmail.com>");
-MODULE_DESCRIPTION("Analog Devices ADE7854/58/68/78 Polyphase Multifunction Energy Metering IC Driver");
+MODULE_DESCRIPTION("Analog Devices ADE7854/58/68/78 Polyphase Energy Meter");
 MODULE_LICENSE("GPL v2");
diff --git a/drivers/staging/iio/meter/ade7854.h b/drivers/staging/iio/meter/ade7854.h
index 47690e5..4ad84a3 100644
--- a/drivers/staging/iio/meter/ade7854.h
+++ b/drivers/staging/iio/meter/ade7854.h
@@ -147,11 +147,7 @@
 /**
  * struct ade7854_state - device instance specific data
  * @spi:			actual spi_device
- * @work_trigger_to_ring: bh for triggered event handling
- * @inter:		used to check if new interrupt has been triggered
- * @last_timestamp:	passing timestamp from th to bh of interrupt handler
  * @indio_dev:		industrial I/O device structure
- * @trig:		data ready trigger registered with iio
  * @tx:			transmit buffer
  * @rx:			recieve buffer
  * @buf_lock:		mutex to protect tx and rx
@@ -159,10 +155,7 @@
 struct ade7854_state {
 	struct spi_device		*spi;
 	struct i2c_client               *i2c;
-	struct work_struct		work_trigger_to_ring;
-	s64				last_timestamp;
 	struct iio_dev			*indio_dev;
-	struct iio_trigger		*trig;
 	u8				*tx;
 	u8				*rx;
 	int				(*read_reg_8) (struct device *, u16, u8 *);
@@ -180,66 +173,4 @@ struct ade7854_state {
 extern int ade7854_probe(struct ade7854_state *st, struct device *dev);
 extern int ade7854_remove(struct ade7854_state *st);
 
-#if defined(CONFIG_IIO_RING_BUFFER) && defined(THIS_HAS_RING_BUFFER_SUPPORT)
-/* At the moment triggers are only used for ring buffer
- * filling. This may change!
- */
-
-enum ade7854_scan {
-	ADE7854_SCAN_PHA_V,
-	ADE7854_SCAN_PHB_V,
-	ADE7854_SCAN_PHC_V,
-	ADE7854_SCAN_PHA_I,
-	ADE7854_SCAN_PHB_I,
-	ADE7854_SCAN_PHC_I,
-};
-
-void ade7854_remove_trigger(struct iio_dev *indio_dev);
-int ade7854_probe_trigger(struct iio_dev *indio_dev);
-
-ssize_t ade7854_read_data_from_ring(struct device *dev,
-				      struct device_attribute *attr,
-				      char *buf);
-
-
-int ade7854_configure_ring(struct iio_dev *indio_dev);
-void ade7854_unconfigure_ring(struct iio_dev *indio_dev);
-
-int ade7854_initialize_ring(struct iio_ring_buffer *ring);
-void ade7854_uninitialize_ring(struct iio_ring_buffer *ring);
-#else /* CONFIG_IIO_RING_BUFFER */
-
-static inline void ade7854_remove_trigger(struct iio_dev *indio_dev)
-{
-}
-static inline int ade7854_probe_trigger(struct iio_dev *indio_dev)
-{
-	return 0;
-}
-
-static inline ssize_t
-ade7854_read_data_from_ring(struct device *dev,
-			      struct device_attribute *attr,
-			      char *buf)
-{
-	return 0;
-}
-
-static inline int ade7854_configure_ring(struct iio_dev *indio_dev)
-{
-	return 0;
-}
-
-static inline void ade7854_unconfigure_ring(struct iio_dev *indio_dev)
-{
-}
-static inline int ade7854_initialize_ring(struct iio_ring_buffer *ring)
-{
-	return 0;
-}
-static inline void ade7854_uninitialize_ring(struct iio_ring_buffer *ring)
-{
-}
-#endif /* CONFIG_IIO_RING_BUFFER */
-
 #endif
-- 
1.7.3.4

RE: [PATCH 0/4] staging:iio:meter clean up and dead code removal.

[Hennerich, Michael]

Jonathan Cameron wrote on 2011-02-05:
> This is very much the starting point for bashing the meter drivers
> into shape.  For now I've just removed most of the unused code and stubs.

Instead of removing the stubs - I would rather prefer implementing them.
But I think you're right this dead code is not going to be useful either.

> I have also cleaned up spi bus handling.  I may well have gotten that
> bit wrong so please do take a look.  In some cases this involved
> turning one transfer into two so as to clarify what the code was doing.
> This may cause issues if the spi bus drivers don't play the game
> properly or as if I got the conversion wrong.

Looks right - but I had to double check with the datasheet.
The SPI write then read behavior is a bit undefined.
In particular, what is shifted out on the MOSI pin during the receive part.

Some implementations send zero/one or repeat the last word in the shift reg=
ister, while others tristate MOSI.
If the target slave device in questions also listens on MOSI while it drive=
s MISO,
unpredicted behavior my result.

It's therefore sometimes necessary to use full duplex transfer and drive
MOSI to a known good state. You basically removed this, but the ADExxxx
Parts don't seem to depend on this.

> Next job is to pin down, standardise and document the sysfs interface
> for these parts.  Anyone care to take this on?

Considering the complexity and number of new arguments, this is going to ta=
ke some time.
But it has to be done. Unfortunately right now I'm a bit loaded with other =
stuff.
But I'll open a tracker item for me to look at.

> Note I have currently left the ade7758 driver alone as it is by far
> the most complex and didn't suffer the large amount of dead code to be
> found in the others.
>
> All comments as ever welcome.  I'd also like to know what test
> coverage we have on these parts.

The ADE7758 is the only one I have test hardware. Ideally we bring this one
in a good shape and add support for the other parts to it.

> Thanks,
>
> Jonathan
>
> Jonathan Cameron (4):
>   staging:iio:meter remove stubs from ade7753.
>   staging:iio:meter remove stubs from ade7754.
>   staging:iio:meter remove stubs from ade7759.
>   staging:iio:meter remove stubs from ade7854.
 drivers/staging/iio/meter/ade7753.c     |  200 +++++++----------------
 -------- drivers/staging/iio/meter/ade7753.h     |   64 ----------
 drivers/staging/iio/meter/ade7754.c     |  165 ++++-------------------

Greetings,
Michael

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

Re: [PATCH 0/4] staging:iio:meter clean up and dead code removal.

[Jonathan Cameron]

On 02/07/11 15:34, Hennerich, Michael wrote:
> Jonathan Cameron wrote on 2011-02-05:
>> This is very much the starting point for bashing the meter drivers
>> into shape.  For now I've just removed most of the unused code and stubs.
> 
> Instead of removing the stubs - I would rather prefer implementing them.
I agree in the ideal case.  But even then, it is much easier to review a patch
that adds this functionality from scratch than it is to review one that relies
on large amounts of existing dead code.
> But I think you're right this dead code is not going to be useful either.
> 
>> I have also cleaned up spi bus handling.  I may well have gotten that
>> bit wrong so please do take a look.  In some cases this involved
>> turning one transfer into two so as to clarify what the code was doing.
>> This may cause issues if the spi bus drivers don't play the game
>> properly or as if I got the conversion wrong.
> 
> Looks right - but I had to double check with the datasheet.
> The SPI write then read behavior is a bit undefined.
> In particular, what is shifted out on the MOSI pin during the receive part.
> 
> Some implementations send zero/one or repeat the last word in the shift register, while others tristate MOSI.
> If the target slave device in questions also listens on MOSI while it drives MISO,
> unpredicted behavior my result.
Strange, I thought this was defined as always sending zero if not provided.  If not
then I would suggest we point this inconsistency out to the spi guys and fix it up
in those implementations that don't conform.  Otherwise as you say, one needs
to always specify the other direction just to make sure nothing weird happens.
All my testing is on ancient pxa boards, so I know that they always send zeros and
dump whatever is recieved.
> 
> It's therefore sometimes necessary to use full duplex transfer and drive
> MOSI to a known good state. You basically removed this, but the ADExxxx
> Parts don't seem to depend on this.
> 
>> Next job is to pin down, standardise and document the sysfs interface
>> for these parts.  Anyone care to take this on?
> 
> Considering the complexity and number of new arguments, this is going to take some time.
> But it has to be done. Unfortunately right now I'm a bit loaded with other stuff.
> But I'll open a tracker item for me to look at.
Cool.  If I get really board I might do a data sheet trawl and try and pin this down
in the meantime.
> 
>> Note I have currently left the ade7758 driver alone as it is by far
>> the most complex and didn't suffer the large amount of dead code to be
>> found in the others.
>>
>> All comments as ever welcome.  I'd also like to know what test
>> coverage we have on these parts.
> 
> The ADE7758 is the only one I have test hardware. Ideally we bring this one
> in a good shape and add support for the other parts to it.
Works for me.  Though they are pretty different so I'm not sure a unified driver
would work terribly well for these.  Could be wrong though :)
> 
>> Thanks,
>>
>> Jonathan
>>
>> Jonathan Cameron (4):
>>   staging:iio:meter remove stubs from ade7753.
>>   staging:iio:meter remove stubs from ade7754.
>>   staging:iio:meter remove stubs from ade7759.
>>   staging:iio:meter remove stubs from ade7854.
>  drivers/staging/iio/meter/ade7753.c     |  200 +++++++----------------
>  -------- drivers/staging/iio/meter/ade7753.h     |   64 ----------
>  drivers/staging/iio/meter/ade7754.c     |  165 ++++-------------------
> 
> Greetings,
> Michael
> 
> --
> Analog Devices GmbH      Wilhelm-Wagenfeld-Str. 6      80807 Muenchen
> Sitz der Gesellschaft: Muenchen; Registergericht: Muenchen HRB 40368; Geschaeftsfuehrer:Dr.Carsten Suckrow, Thomas Wessel, William A. Martin, Margaret Seif
> 
> 
> 

RE: [PATCH 0/4] staging:iio:meter clean up and dead code removal.

[Hennerich, Michael]

Jonathan Cameron wrote on 2011-02-07:
> On 02/07/11 15:34, Hennerich, Michael wrote:
>> Jonathan Cameron wrote on 2011-02-05:
>>> This is very much the starting point for bashing the meter drivers
>>> into shape.  For now I've just removed most of the unused code and
> stubs.
>>
>> Instead of removing the stubs - I would rather prefer implementing
> them.
> I agree in the ideal case.  But even then, it is much easier to review
> a patch that adds this functionality from scratch than it is to review
> one that relies on large amounts of existing dead code.
>> But I think you're right this dead code is not going to be useful
>> either.
>>
>>> I have also cleaned up spi bus handling.  I may well have gotten that
>>> bit wrong so please do take a look.  In some cases this involved
>>> turning one transfer into two so as to clarify what the code was
>>> doing. This may cause issues if the spi bus drivers don't play the
>>> game properly or as if I got the conversion wrong.
>>
>> Looks right - but I had to double check with the datasheet. The SPI
>> write then read behavior is a bit undefined. In particular, what is
>> shifted out on the MOSI pin during the receive part.
>>
>> Some implementations send zero/one or repeat the last word in the shift
>> register, while others tristate MOSI. If the target slave device in
>> questions also listens on MOSI while it drives MISO, unpredicted
>> behavior my result.
> Strange, I thought this was defined as always sending zero if not
> provided.  If not then I would suggest we point this inconsistency out
> to the spi guys and fix it up in those implementations that don't
> conform.  Otherwise as you say, one needs to always specify the other
> direction just to make sure nothing weird happens.
> All my testing is on ancient pxa boards, so I know that they always
> send zeros and dump whatever is recieved.

This behavior is implemented in the SPI Master Controller Drivers, and not =
in the SPI Bus Core.
I know in the past some machine dependant implementations did it wrong.
However looking at the drivers today they all (at least the ones I looked a=
t)
send zero in case tx_buf is NULL.

>>
>> It's therefore sometimes necessary to use full duplex transfer and
>> drive MOSI to a known good state. You basically removed this, but
>> the ADExxxx Parts don't seem to depend on this.
>>
>>> Next job is to pin down, standardise and document the sysfs interface
>>> for these parts.  Anyone care to take this on?
>>
>> Considering the complexity and number of new arguments, this is going
>> to take some time. But it has to be done. Unfortunately right now I'm a
>> bit loaded with other stuff. But I'll open a tracker item for me to
>> look at.
> Cool.  If I get really board I might do a data sheet trawl and try and
> pin this down in the meantime.

Hope you get board anytime soon. :-)
No just kidding. If you want to take the lead here,feel free, and I also ca=
n get you
an evaluation board. But I don't expect you to work on this.

>>
>>> Note I have currently left the ade7758 driver alone as it is by far
>>> the most complex and didn't suffer the large amount of dead code to
>>> be found in the others.
>>>
>>> All comments as ever welcome.  I'd also like to know what test
>>> coverage we have on these parts.
>>
>> The ADE7758 is the only one I have test hardware. Ideally we bring
>> this one in a good shape and add support for the other parts to it.
> Works for me.  Though they are pretty different so I'm not sure a
> unified driver would work terribly well for these.  Could be wrong
> though :)

Folding all in one driver probably get's a bit messy. Whatever makes the
best sense, while preserving readability and maintainability.

Thanks,
Michael


>>
>>> Thanks,
>>>
>>> Jonathan
>>>
>>> Jonathan Cameron (4):
>>>   staging:iio:meter remove stubs from ade7753.
>>>   staging:iio:meter remove stubs from ade7754.
>>>   staging:iio:meter remove stubs from ade7759.
>>>   staging:iio:meter remove stubs from ade7854.
>>  drivers/staging/iio/meter/ade7753.c     |  200 +++++++--------------

Greetings,
Michael

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

Re: [PATCH 0/4] staging:iio:meter clean up and dead code removal.

[Jonathan Cameron]

On 02/08/11 09:32, Hennerich, Michael wrote:
> Jonathan Cameron wrote on 2011-02-07:
>> On 02/07/11 15:34, Hennerich, Michael wrote:
>>> Jonathan Cameron wrote on 2011-02-05:
>>>> This is very much the starting point for bashing the meter drivers
>>>> into shape.  For now I've just removed most of the unused code and
>> stubs.
>>>
>>> Instead of removing the stubs - I would rather prefer implementing
>> them.
>> I agree in the ideal case.  But even then, it is much easier to review
>> a patch that adds this functionality from scratch than it is to review
>> one that relies on large amounts of existing dead code.
>>> But I think you're right this dead code is not going to be useful
>>> either.
>>>
>>>> I have also cleaned up spi bus handling.  I may well have gotten that
>>>> bit wrong so please do take a look.  In some cases this involved
>>>> turning one transfer into two so as to clarify what the code was
>>>> doing. This may cause issues if the spi bus drivers don't play the
>>>> game properly or as if I got the conversion wrong.
>>>
>>> Looks right - but I had to double check with the datasheet. The SPI
>>> write then read behavior is a bit undefined. In particular, what is
>>> shifted out on the MOSI pin during the receive part.
>>>
>>> Some implementations send zero/one or repeat the last word in the shift
>>> register, while others tristate MOSI. If the target slave device in
>>> questions also listens on MOSI while it drives MISO, unpredicted
>>> behavior my result.
>> Strange, I thought this was defined as always sending zero if not
>> provided.  If not then I would suggest we point this inconsistency out
>> to the spi guys and fix it up in those implementations that don't
>> conform.  Otherwise as you say, one needs to always specify the other
>> direction just to make sure nothing weird happens.
>> All my testing is on ancient pxa boards, so I know that they always
>> send zeros and dump whatever is recieved.
> 
> This behavior is implemented in the SPI Master Controller Drivers, and not in the SPI Bus Core.
> I know in the past some machine dependant implementations did it wrong.
> However looking at the drivers today they all (at least the ones I looked at)
> send zero in case tx_buf is NULL.
Good. In that case shall I send this set on to Greg? Might as well get the
easy cleanups in place asap.

....

RE: [PATCH 0/4] staging:iio:meter clean up and dead code removal.

[Hennerich, Michael]

Jonathan Cameron wrote on 2011-02-08:
> On 02/08/11 09:32, Hennerich, Michael wrote:
>> Jonathan Cameron wrote on 2011-02-07:
>>> On 02/07/11 15:34, Hennerich, Michael wrote:
>>>> Jonathan Cameron wrote on 2011-02-05:
>>>>> This is very much the starting point for bashing the meter
>>>>> drivers into shape.  For now I've just removed most of the unused
>>>>> code and
>>> stubs.
>>>>
>>>> Instead of removing the stubs - I would rather prefer implementing
>>> them.
>>> I agree in the ideal case.  But even then, it is much easier to
>>> review a patch that adds this functionality from scratch than it is
>>> to review one that relies on large amounts of existing dead code.
>>>> But I think you're right this dead code is not going to be useful
>>>> either.
>>>>
>>>>> I have also cleaned up spi bus handling.  I may well have gotten
>>>>> that bit wrong so please do take a look.  In some cases this
>>>>> involved turning one transfer into two so as to clarify what the
>>>>> code was doing. This may cause issues if the spi bus drivers
>>>>> don't play the game properly or as if I got the conversion wrong.
>>>>
>>>> Looks right - but I had to double check with the datasheet. The
>>>> SPI write then read behavior is a bit undefined. In particular,
>>>> what is shifted out on the MOSI pin during the receive part.
>>>>
>>>> Some implementations send zero/one or repeat the last word in the
>>>> shift register, while others tristate MOSI. If the target slave
>>>> device in questions also listens on MOSI while it drives MISO,
>>>> unpredicted behavior my result.
>>> Strange, I thought this was defined as always sending zero if not
>>> provided.  If not then I would suggest we point this inconsistency out
>>> to the spi guys and fix it up in those implementations that don't
>>> conform.  Otherwise as you say, one needs to always specify the other
>>> direction just to make sure nothing weird happens. All my testing is
>>> on ancient pxa boards, so I know that they always send zeros and dump
>>> whatever is recieved.
>>
>> This behavior is implemented in the SPI Master Controller Drivers, and
>> not in the SPI Bus Core. I know in the past some machine dependant
>> implementations did it wrong. However looking at the drivers today they
>> all (at least the ones I looked at) send zero in case tx_buf is NULL.
> Good. In that case shall I send this set on to Greg? Might as well get
> the easy cleanups in place asap.

Yeah please.

Acked-by: Michael Hennerich <michael.hennerich@analog.com>

Greetings,
Michael

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