[PATCH 00/13] staging:iio:ad7606: Towards moving out of staging

[PATCH 00/13] staging:iio:ad7606: Towards moving out of staging

[Lars-Peter Clausen]

Sorry, if you received this twice, forgot the list on the first try.

This patch series contains a few cleanups for the ad7606 driver that
prepares it for moving out of staging. There are still a few small things
left to do after this series though, but hopefully we can get to them
within this developement window so the driver can be moved for the final
v4.10 IIO pull request.

Among the things that still need to be addressed:
	* Replace range/range_available with the standard scale/scale_available
	* Cleanup the regulator handling so it matches the standard behavior
	  established by the framework

- Lars

Lars-Peter Clausen (13):
  staging:iio:ad7606: Remove unused int_vref_mv field
  staging:iio:ad7606: Remove redundant name field from ad7606_chip_info
  staging:iio:ad7606: Remove default device configuration from platform
    data
  staging:iio:ad7606: Remove out-of-band error reporting
  staging:iio:ad7606: Use oversampling ratio of 1 for no oversampling
  staging:iio:ad7606: Avoid allocating buffer for each data capture
  staging:iio:ad7606: Factor out common code between periodic and
    one-shot capture
  staging:iio:ad7606: Move set_drvdata() into common code
  staging:iio:ad7606: Let the common probe function return int
  staging:iio:ad7606: Let common remove function take a struct device *
  staging:iio:ad7606: Run trigger handler only once per trigger event
  staging:iio:ad7606: Use GPIO descriptor API
  staging:iio:ad7606: Move buffer code to main source file

 drivers/staging/iio/adc/Makefile      |   1 -
 drivers/staging/iio/adc/ad7606.c      | 545 +++++++++++++++++++++++++++++++
 drivers/staging/iio/adc/ad7606.h      |  58 +---
 drivers/staging/iio/adc/ad7606_core.c | 598 ----------------------------------
 drivers/staging/iio/adc/ad7606_par.c  |  23 +-
 drivers/staging/iio/adc/ad7606_ring.c | 102 ------
 drivers/staging/iio/adc/ad7606_spi.c  |  19 +-
 7 files changed, 569 insertions(+), 777 deletions(-)
 create mode 100644 drivers/staging/iio/adc/ad7606.c
 delete mode 100644 drivers/staging/iio/adc/ad7606_core.c
 delete mode 100644 drivers/staging/iio/adc/ad7606_ring.c

-- 
2.1.4

[PATCH 01/13] staging:iio:ad7606: Remove unused int_vref_mv field

[Lars-Peter Clausen]

Remove the int_vref_mv field from the ad7606_chip_info struct since the
field is never used by the driver. The value is also the same for all
derivatives of this chip, so if it will ever be used in the driver a
constant value will work just fine.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/staging/iio/adc/ad7606.h      | 2 --
 drivers/staging/iio/adc/ad7606_core.c | 3 ---
 2 files changed, 5 deletions(-)

diff --git a/drivers/staging/iio/adc/ad7606.h b/drivers/staging/iio/adc/ad7606.h
index 39f5044..b7e59a6 100644
--- a/drivers/staging/iio/adc/ad7606.h
+++ b/drivers/staging/iio/adc/ad7606.h
@@ -43,14 +43,12 @@ struct ad7606_platform_data {
 /**
  * struct ad7606_chip_info - chip specific information
  * @name:		identification string for chip
- * @int_vref_mv:	the internal reference voltage
  * @channels:		channel specification
  * @num_channels:	number of channels
  */
 
 struct ad7606_chip_info {
 	const char			*name;
-	u16				int_vref_mv;
 	const struct iio_chan_spec	*channels;
 	unsigned int			num_channels;
 };
diff --git a/drivers/staging/iio/adc/ad7606_core.c b/drivers/staging/iio/adc/ad7606_core.c
index 2042225..a1f18d4 100644
--- a/drivers/staging/iio/adc/ad7606_core.c
+++ b/drivers/staging/iio/adc/ad7606_core.c
@@ -261,19 +261,16 @@ static const struct ad7606_chip_info ad7606_chip_info_tbl[] = {
 	 */
 	[ID_AD7606_8] = {
 		.name = "ad7606",
-		.int_vref_mv = 2500,
 		.channels = ad7606_channels,
 		.num_channels = 9,
 	},
 	[ID_AD7606_6] = {
 		.name = "ad7606-6",
-		.int_vref_mv = 2500,
 		.channels = ad7606_channels,
 		.num_channels = 7,
 	},
 	[ID_AD7606_4] = {
 		.name = "ad7606-4",
-		.int_vref_mv = 2500,
 		.channels = ad7606_channels,
 		.num_channels = 5,
 	},
-- 
2.1.4

[PATCH 02/13] staging:iio:ad7606: Remove redundant name field from ad7606_chip_info

[Lars-Peter Clausen]

The name field in the ad7606_chip_info struct is set to the same value as
the as the name field in the corresponding {platform,spi}_device_id table
entry. Remove it from the ad7606_chip_info struct and pass the name from
the ID to the probe function. This slightly reduces the size of the
chip_info table and adding new entries requires less boilerplate.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/staging/iio/adc/ad7606.h      |  4 ++--
 drivers/staging/iio/adc/ad7606_core.c | 11 ++++-------
 drivers/staging/iio/adc/ad7606_par.c  |  3 ++-
 drivers/staging/iio/adc/ad7606_spi.c  |  3 ++-
 4 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/iio/adc/ad7606.h b/drivers/staging/iio/adc/ad7606.h
index b7e59a6..380c56a 100644
--- a/drivers/staging/iio/adc/ad7606.h
+++ b/drivers/staging/iio/adc/ad7606.h
@@ -48,7 +48,6 @@ struct ad7606_platform_data {
  */
 
 struct ad7606_chip_info {
-	const char			*name;
 	const struct iio_chan_spec	*channels;
 	unsigned int			num_channels;
 };
@@ -84,7 +83,8 @@ struct ad7606_bus_ops {
 };
 
 struct iio_dev *ad7606_probe(struct device *dev, int irq,
-			      void __iomem *base_address, unsigned int id,
+			      void __iomem *base_address,
+			      const char *name, unsigned int id,
 			      const struct ad7606_bus_ops *bops);
 int ad7606_remove(struct iio_dev *indio_dev, int irq);
 int ad7606_reset(struct ad7606_state *st);
diff --git a/drivers/staging/iio/adc/ad7606_core.c b/drivers/staging/iio/adc/ad7606_core.c
index a1f18d4..1093647 100644
--- a/drivers/staging/iio/adc/ad7606_core.c
+++ b/drivers/staging/iio/adc/ad7606_core.c
@@ -260,17 +260,14 @@ static const struct ad7606_chip_info ad7606_chip_info_tbl[] = {
 	 * More devices added in future
 	 */
 	[ID_AD7606_8] = {
-		.name = "ad7606",
 		.channels = ad7606_channels,
 		.num_channels = 9,
 	},
 	[ID_AD7606_6] = {
-		.name = "ad7606-6",
 		.channels = ad7606_channels,
 		.num_channels = 7,
 	},
 	[ID_AD7606_4] = {
-		.name = "ad7606-4",
 		.channels = ad7606_channels,
 		.num_channels = 5,
 	},
@@ -438,7 +435,7 @@ static const struct iio_info ad7606_info_range = {
 
 struct iio_dev *ad7606_probe(struct device *dev, int irq,
 			     void __iomem *base_address,
-			     unsigned int id,
+			     const char *name, unsigned int id,
 			     const struct ad7606_bus_ops *bops)
 {
 	struct ad7606_platform_data *pdata = dev->platform_data;
@@ -491,7 +488,7 @@ struct iio_dev *ad7606_probe(struct device *dev, int irq,
 			indio_dev->info = &ad7606_info_no_os_or_range;
 	}
 	indio_dev->modes = INDIO_DIRECT_MODE;
-	indio_dev->name = st->chip_info->name;
+	indio_dev->name = name;
 	indio_dev->channels = st->chip_info->channels;
 	indio_dev->num_channels = st->chip_info->num_channels;
 
@@ -505,8 +502,8 @@ struct iio_dev *ad7606_probe(struct device *dev, int irq,
 	if (ret)
 		dev_warn(st->dev, "failed to RESET: no RESET GPIO specified\n");
 
-	ret = request_irq(irq, ad7606_interrupt,
-			  IRQF_TRIGGER_FALLING, st->chip_info->name, indio_dev);
+	ret = request_irq(irq, ad7606_interrupt, IRQF_TRIGGER_FALLING, name,
+			  indio_dev);
 	if (ret)
 		goto error_free_gpios;
 
diff --git a/drivers/staging/iio/adc/ad7606_par.c b/drivers/staging/iio/adc/ad7606_par.c
index 84d2393..0a23cd9 100644
--- a/drivers/staging/iio/adc/ad7606_par.c
+++ b/drivers/staging/iio/adc/ad7606_par.c
@@ -49,6 +49,7 @@ static const struct ad7606_bus_ops ad7606_par8_bops = {
 
 static int ad7606_par_probe(struct platform_device *pdev)
 {
+	const struct platform_device_id *id = platform_get_device_id(pdev);
 	struct resource *res;
 	struct iio_dev *indio_dev;
 	void __iomem *addr;
@@ -69,7 +70,7 @@ static int ad7606_par_probe(struct platform_device *pdev)
 	remap_size = resource_size(res);
 
 	indio_dev = ad7606_probe(&pdev->dev, irq, addr,
-				 platform_get_device_id(pdev)->driver_data,
+				 id->name, id->driver_data,
 				 remap_size > 1 ? &ad7606_par16_bops :
 				 &ad7606_par8_bops);
 
diff --git a/drivers/staging/iio/adc/ad7606_spi.c b/drivers/staging/iio/adc/ad7606_spi.c
index 9587fa8..f69db59 100644
--- a/drivers/staging/iio/adc/ad7606_spi.c
+++ b/drivers/staging/iio/adc/ad7606_spi.c
@@ -42,10 +42,11 @@ static const struct ad7606_bus_ops ad7606_spi_bops = {
 
 static int ad7606_spi_probe(struct spi_device *spi)
 {
+	const struct spi_device_id *id = spi_get_device_id(spi);
 	struct iio_dev *indio_dev;
 
 	indio_dev = ad7606_probe(&spi->dev, spi->irq, NULL,
-				 spi_get_device_id(spi)->driver_data,
+				 id->name, id->driver_data,
 				 &ad7606_spi_bops);
 
 	if (IS_ERR(indio_dev))
-- 
2.1.4

[PATCH 03/13] staging:iio:ad7606: Remove default device configuration from platform data

[Lars-Peter Clausen]

While for some very selected setups it might be useful to be able to
provide default configuration data via the platform data, generally this
becomes very impractical as the number of configuration options increases.
So the general policy is to use the power-on default values of the device
and let the application using the device configure it according to its
needs.

Implement this scheme for the ad7606 driver by removing support for
specifying a default configuration via the platform data.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/staging/iio/adc/ad7606.h      |  4 ----
 drivers/staging/iio/adc/ad7606_core.c | 12 ++----------
 2 files changed, 2 insertions(+), 14 deletions(-)

diff --git a/drivers/staging/iio/adc/ad7606.h b/drivers/staging/iio/adc/ad7606.h
index 380c56a..cf6be65 100644
--- a/drivers/staging/iio/adc/ad7606.h
+++ b/drivers/staging/iio/adc/ad7606.h
@@ -15,8 +15,6 @@
 
 /**
  * struct ad7606_platform_data - platform/board specific information
- * @default_os:		default oversampling value {0, 2, 4, 8, 16, 32, 64}
- * @default_range:	default range +/-{5000, 10000} mVolt
  * @gpio_convst:	number of gpio connected to the CONVST pin
  * @gpio_reset:		gpio connected to the RESET pin, if not used set to -1
  * @gpio_range:		gpio connected to the RANGE pin, if not used set to -1
@@ -28,8 +26,6 @@
  */
 
 struct ad7606_platform_data {
-	unsigned int			default_os;
-	unsigned int			default_range;
 	unsigned int			gpio_convst;
 	unsigned int			gpio_reset;
 	unsigned int			gpio_range;
diff --git a/drivers/staging/iio/adc/ad7606_core.c b/drivers/staging/iio/adc/ad7606_core.c
index 1093647..a16c6f5 100644
--- a/drivers/staging/iio/adc/ad7606_core.c
+++ b/drivers/staging/iio/adc/ad7606_core.c
@@ -452,16 +452,8 @@ struct iio_dev *ad7606_probe(struct device *dev, int irq,
 	st->dev = dev;
 	st->bops = bops;
 	st->base_address = base_address;
-	st->range = pdata->default_range == 10000 ? 10000 : 5000;
-
-	ret = ad7606_oversampling_get_index(pdata->default_os);
-	if (ret < 0) {
-		dev_warn(dev, "oversampling %d is not supported\n",
-			 pdata->default_os);
-		st->oversampling = 0;
-	} else {
-		st->oversampling = pdata->default_os;
-	}
+	st->range = 5000;
+	st->oversampling = 0;
 
 	st->reg = devm_regulator_get(dev, "vcc");
 	if (!IS_ERR(st->reg)) {
-- 
2.1.4

[PATCH 04/13] staging:iio:ad7606: Remove out-of-band error reporting

[Lars-Peter Clausen]

Currently the ad7606 driver prints a error message to the kernel log when
an application writes an invalid value to a sysfs attribute. While for
initial driver development and testing this might be useful it is quite
disadvantageous in a production environment. The write() call to the sysfs
attribute will already return an error if the value was invalid so the
application is aware that the operation failed. And generally speaking it
is impossible for an application to reliably match a log message in the
kernel log to a specific operation it performed, so the message becomes
just noise and might distract from more critical messages.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/staging/iio/adc/ad7606_core.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/iio/adc/ad7606_core.c b/drivers/staging/iio/adc/ad7606_core.c
index a16c6f5..d3a3551 100644
--- a/drivers/staging/iio/adc/ad7606_core.c
+++ b/drivers/staging/iio/adc/ad7606_core.c
@@ -132,10 +132,9 @@ static ssize_t ad7606_store_range(struct device *dev,
 	if (ret)
 		return ret;
 
-	if (!(lval == 5000 || lval == 10000)) {
-		dev_err(dev, "range is not supported\n");
+	if (!(lval == 5000 || lval == 10000))
 		return -EINVAL;
-	}
+
 	mutex_lock(&indio_dev->mlock);
 	gpio_set_value(st->pdata->gpio_range, lval == 10000);
 	st->range = lval;
@@ -174,11 +173,8 @@ static int ad7606_write_raw(struct iio_dev *indio_dev,
 		if (val2)
 			return -EINVAL;
 		ret = ad7606_oversampling_get_index(val);
-		if (ret < 0) {
-			dev_err(st->dev, "oversampling %d is not supported\n",
-				val);
+		if (ret < 0)
 			return ret;
-		}
 
 		mutex_lock(&indio_dev->mlock);
 		gpio_set_value(st->pdata->gpio_os0, (ret >> 0) & 1);
-- 
2.1.4

[PATCH 05/13] staging:iio:ad7606: Use oversampling ratio of 1 for no oversampling

[Lars-Peter Clausen]

Currently the ad7606 driver uses a value of 0 for the oversampling ratio to
express that no oversampling is done. Strictly speaking this means though
that no data capture is done at all. Instead change the driver to use a
value of 1, this is in accordance with what other drivers do and what the
IIO spec suggests.

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

diff --git a/drivers/staging/iio/adc/ad7606_core.c b/drivers/staging/iio/adc/ad7606_core.c
index d3a3551..7c093e2 100644
--- a/drivers/staging/iio/adc/ad7606_core.c
+++ b/drivers/staging/iio/adc/ad7606_core.c
@@ -149,7 +149,7 @@ static IIO_CONST_ATTR(in_voltage_range_available, "5000 10000");
 
 static int ad7606_oversampling_get_index(unsigned int val)
 {
-	unsigned char supported[] = {0, 2, 4, 8, 16, 32, 64};
+	unsigned char supported[] = {1, 2, 4, 8, 16, 32, 64};
 	int i;
 
 	for (i = 0; i < ARRAY_SIZE(supported); i++)
@@ -188,7 +188,7 @@ static int ad7606_write_raw(struct iio_dev *indio_dev,
 	}
 }
 
-static IIO_CONST_ATTR(oversampling_ratio_available, "0 2 4 8 16 32 64");
+static IIO_CONST_ATTR(oversampling_ratio_available, "1 2 4 8 16 32 64");
 
 static struct attribute *ad7606_attributes_os_and_range[] = {
 	&iio_dev_attr_in_voltage_range.dev_attr.attr,
@@ -449,7 +449,7 @@ struct iio_dev *ad7606_probe(struct device *dev, int irq,
 	st->bops = bops;
 	st->base_address = base_address;
 	st->range = 5000;
-	st->oversampling = 0;
+	st->oversampling = 1;
 
 	st->reg = devm_regulator_get(dev, "vcc");
 	if (!IS_ERR(st->reg)) {
-- 
2.1.4

[PATCH 06/13] staging:iio:ad7606: Avoid allocating buffer for each data capture

[Lars-Peter Clausen]

Currently the ad7606 driver dynamically allocates and frees a transfer
buffer each time a sample capture is performed in buffered mode, which
introduces unnecessary overhead. The driver state struct already contains a
buffer that is used for transfers in one-shot mode. This buffer is large
enough to hold all samples, but not the timestamp that might be present in
buffered mode. Extend the buffer size to be able to contain the timestamp
and update the buffered capture function to use this buffer.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/staging/iio/adc/ad7606.h      |  4 ++--
 drivers/staging/iio/adc/ad7606_ring.c | 14 ++++----------
 2 files changed, 6 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/iio/adc/ad7606.h b/drivers/staging/iio/adc/ad7606.h
index cf6be65..129f94e 100644
--- a/drivers/staging/iio/adc/ad7606.h
+++ b/drivers/staging/iio/adc/ad7606.h
@@ -68,9 +68,9 @@ struct ad7606_state {
 	/*
 	 * DMA (thus cache coherency maintenance) requires the
 	 * transfer buffers to live in their own cache lines.
+	 * 8 * 16-bit samples + 64-bit timestamp
 	 */
-
-	unsigned short			data[8] ____cacheline_aligned;
+	unsigned short			data[12] ____cacheline_aligned;
 };
 
 struct ad7606_bus_ops {
diff --git a/drivers/staging/iio/adc/ad7606_ring.c b/drivers/staging/iio/adc/ad7606_ring.c
index 0572df9..7fa4ccc 100644
--- a/drivers/staging/iio/adc/ad7606_ring.c
+++ b/drivers/staging/iio/adc/ad7606_ring.c
@@ -46,15 +46,10 @@ static void ad7606_poll_bh_to_ring(struct work_struct *work_s)
 	struct ad7606_state *st = container_of(work_s, struct ad7606_state,
 						poll_work);
 	struct iio_dev *indio_dev = iio_priv_to_dev(st);
-	__u8 *buf;
 	int ret;
 
-	buf = kzalloc(indio_dev->scan_bytes, GFP_KERNEL);
-	if (!buf)
-		return;
-
 	if (gpio_is_valid(st->pdata->gpio_frstdata)) {
-		ret = st->bops->read_block(st->dev, 1, buf);
+		ret = st->bops->read_block(st->dev, 1, st->data);
 		if (ret)
 			goto done;
 		if (!gpio_get_value(st->pdata->gpio_frstdata)) {
@@ -67,22 +62,21 @@ static void ad7606_poll_bh_to_ring(struct work_struct *work_s)
 			goto done;
 		}
 		ret = st->bops->read_block(st->dev,
-			st->chip_info->num_channels - 1, buf + 2);
+			st->chip_info->num_channels - 1, st->data + 1);
 		if (ret)
 			goto done;
 	} else {
 		ret = st->bops->read_block(st->dev,
-			st->chip_info->num_channels, buf);
+			st->chip_info->num_channels, st->data);
 		if (ret)
 			goto done;
 	}
 
-	iio_push_to_buffers_with_timestamp(indio_dev, buf,
+	iio_push_to_buffers_with_timestamp(indio_dev, st->data,
 					   iio_get_time_ns(indio_dev));
 done:
 	gpio_set_value(st->pdata->gpio_convst, 0);
 	iio_trigger_notify_done(indio_dev->trig);
-	kfree(buf);
 }
 
 int ad7606_register_ring_funcs_and_init(struct iio_dev *indio_dev)
-- 
2.1.4

[PATCH 07/13] staging:iio:ad7606: Factor out common code between periodic and one-shot capture

[Lars-Peter Clausen]

Both the periodic buffer based and one-shot sysfs based capture methods
share a large portion of their code. Factor this out into a common helper
function.

Also provide a comment that better explains in more detail what is going on
in the capture function.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/staging/iio/adc/ad7606.h      |  1 +
 drivers/staging/iio/adc/ad7606_core.c | 58 ++++++++++++++++++++++-------------
 drivers/staging/iio/adc/ad7606_ring.c | 30 +++---------------
 3 files changed, 41 insertions(+), 48 deletions(-)

diff --git a/drivers/staging/iio/adc/ad7606.h b/drivers/staging/iio/adc/ad7606.h
index 129f94e..2257bdb 100644
--- a/drivers/staging/iio/adc/ad7606.h
+++ b/drivers/staging/iio/adc/ad7606.h
@@ -84,6 +84,7 @@ struct iio_dev *ad7606_probe(struct device *dev, int irq,
 			      const struct ad7606_bus_ops *bops);
 int ad7606_remove(struct iio_dev *indio_dev, int irq);
 int ad7606_reset(struct ad7606_state *st);
+int ad7606_read_samples(struct ad7606_state *st);
 
 enum ad7606_supported_device_ids {
 	ID_AD7606_8,
diff --git a/drivers/staging/iio/adc/ad7606_core.c b/drivers/staging/iio/adc/ad7606_core.c
index 7c093e2..4ce103a 100644
--- a/drivers/staging/iio/adc/ad7606_core.c
+++ b/drivers/staging/iio/adc/ad7606_core.c
@@ -36,6 +36,39 @@ int ad7606_reset(struct ad7606_state *st)
 	return -ENODEV;
 }
 
+int ad7606_read_samples(struct ad7606_state *st)
+{
+	unsigned int num = st->chip_info->num_channels;
+	u16 *data = st->data;
+	int ret;
+
+	/*
+	 * The frstdata signal is set to high while and after reading the sample
+	 * of the first channel and low for all other channels. This can be used
+	 * to check that the incoming data is correctly aligned. During normal
+	 * operation the data should never become unaligned, but some glitch or
+	 * electrostatic discharge might cause an extra read or clock cycle.
+	 * Monitoring the frstdata signal allows to recover from such failure
+	 * situations.
+	 */
+
+	if (gpio_is_valid(st->pdata->gpio_frstdata)) {
+		ret = st->bops->read_block(st->dev, 1, data);
+		if (ret)
+			return ret;
+
+		if (!gpio_get_value(st->pdata->gpio_frstdata)) {
+			ad7606_reset(st);
+			return -EIO;
+		}
+
+		data++;
+		num--;
+	}
+
+	return st->bops->read_block(st->dev, num, data);
+}
+
 static int ad7606_scan_direct(struct iio_dev *indio_dev, unsigned int ch)
 {
 	struct ad7606_state *st = iio_priv(indio_dev);
@@ -48,28 +81,9 @@ static int ad7606_scan_direct(struct iio_dev *indio_dev, unsigned int ch)
 	if (ret)
 		goto error_ret;
 
-	if (gpio_is_valid(st->pdata->gpio_frstdata)) {
-		ret = st->bops->read_block(st->dev, 1, st->data);
-		if (ret)
-			goto error_ret;
-		if (!gpio_get_value(st->pdata->gpio_frstdata)) {
-			/* This should never happen */
-			ad7606_reset(st);
-			ret = -EIO;
-			goto error_ret;
-		}
-		ret = st->bops->read_block(st->dev,
-			st->chip_info->num_channels - 1, &st->data[1]);
-		if (ret)
-			goto error_ret;
-	} else {
-		ret = st->bops->read_block(st->dev,
-			st->chip_info->num_channels, st->data);
-		if (ret)
-			goto error_ret;
-	}
-
-	ret = st->data[ch];
+	ret = ad7606_read_samples(st);
+	if (ret == 0)
+		ret = st->data[ch];
 
 error_ret:
 	gpio_set_value(st->pdata->gpio_convst, 0);
diff --git a/drivers/staging/iio/adc/ad7606_ring.c b/drivers/staging/iio/adc/ad7606_ring.c
index 7fa4ccc..b7bf0cf 100644
--- a/drivers/staging/iio/adc/ad7606_ring.c
+++ b/drivers/staging/iio/adc/ad7606_ring.c
@@ -48,33 +48,11 @@ static void ad7606_poll_bh_to_ring(struct work_struct *work_s)
 	struct iio_dev *indio_dev = iio_priv_to_dev(st);
 	int ret;
 
-	if (gpio_is_valid(st->pdata->gpio_frstdata)) {
-		ret = st->bops->read_block(st->dev, 1, st->data);
-		if (ret)
-			goto done;
-		if (!gpio_get_value(st->pdata->gpio_frstdata)) {
-			/* This should never happen. However
-			 * some signal glitch caused by bad PCB desgin or
-			 * electrostatic discharge, could cause an extra read
-			 * or clock. This allows recovery.
-			 */
-			ad7606_reset(st);
-			goto done;
-		}
-		ret = st->bops->read_block(st->dev,
-			st->chip_info->num_channels - 1, st->data + 1);
-		if (ret)
-			goto done;
-	} else {
-		ret = st->bops->read_block(st->dev,
-			st->chip_info->num_channels, st->data);
-		if (ret)
-			goto done;
-	}
+	ret = ad7606_read_samples(st);
+	if (ret == 0)
+		iio_push_to_buffers_with_timestamp(indio_dev, st->data,
+						   iio_get_time_ns(indio_dev));
 
-	iio_push_to_buffers_with_timestamp(indio_dev, st->data,
-					   iio_get_time_ns(indio_dev));
-done:
 	gpio_set_value(st->pdata->gpio_convst, 0);
 	iio_trigger_notify_done(indio_dev->trig);
 }
-- 
2.1.4

[PATCH 08/13] staging:iio:ad7606: Move set_drvdata() into common code

[Lars-Peter Clausen]

Both the platform_device and SPI driver call set_drvdata() at the end of
their probe function. Move this into the common probe() function to reduce
duplicated code.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/staging/iio/adc/ad7606_core.c | 2 ++
 drivers/staging/iio/adc/ad7606_par.c  | 2 --
 drivers/staging/iio/adc/ad7606_spi.c  | 2 --
 3 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/iio/adc/ad7606_core.c b/drivers/staging/iio/adc/ad7606_core.c
index 4ce103a..13d8090 100644
--- a/drivers/staging/iio/adc/ad7606_core.c
+++ b/drivers/staging/iio/adc/ad7606_core.c
@@ -517,6 +517,8 @@ struct iio_dev *ad7606_probe(struct device *dev, int irq,
 	if (ret)
 		goto error_unregister_ring;
 
+	dev_set_drvdata(dev, indio_dev);
+
 	return indio_dev;
 error_unregister_ring:
 	ad7606_ring_cleanup(indio_dev);
diff --git a/drivers/staging/iio/adc/ad7606_par.c b/drivers/staging/iio/adc/ad7606_par.c
index 0a23cd9..c273993 100644
--- a/drivers/staging/iio/adc/ad7606_par.c
+++ b/drivers/staging/iio/adc/ad7606_par.c
@@ -77,8 +77,6 @@ static int ad7606_par_probe(struct platform_device *pdev)
 	if (IS_ERR(indio_dev))
 		return PTR_ERR(indio_dev);
 
-	platform_set_drvdata(pdev, indio_dev);
-
 	return 0;
 }
 
diff --git a/drivers/staging/iio/adc/ad7606_spi.c b/drivers/staging/iio/adc/ad7606_spi.c
index f69db59..9abbc15 100644
--- a/drivers/staging/iio/adc/ad7606_spi.c
+++ b/drivers/staging/iio/adc/ad7606_spi.c
@@ -52,8 +52,6 @@ static int ad7606_spi_probe(struct spi_device *spi)
 	if (IS_ERR(indio_dev))
 		return PTR_ERR(indio_dev);
 
-	spi_set_drvdata(spi, indio_dev);
-
 	return 0;
 }
 
-- 
2.1.4

[PATCH 09/13] staging:iio:ad7606: Let the common probe function return int

[Lars-Peter Clausen]

The common probe function for the ad7606 currently returns a struct iio_dev
pointer. The returned value is not used by the individual driver probe
functions other than for error checking.

Let the common probe function return a int instead to report the error
value directly (or 0 on success). This allows to simplify the code.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/staging/iio/adc/ad7606.h      |  7 +++----
 drivers/staging/iio/adc/ad7606_core.c | 15 +++++++--------
 drivers/staging/iio/adc/ad7606_par.c  | 14 ++++----------
 drivers/staging/iio/adc/ad7606_spi.c  | 12 +++---------
 4 files changed, 17 insertions(+), 31 deletions(-)

diff --git a/drivers/staging/iio/adc/ad7606.h b/drivers/staging/iio/adc/ad7606.h
index 2257bdb..b4b25a2 100644
--- a/drivers/staging/iio/adc/ad7606.h
+++ b/drivers/staging/iio/adc/ad7606.h
@@ -78,10 +78,9 @@ struct ad7606_bus_ops {
 	int (*read_block)(struct device *, int, void *);
 };
 
-struct iio_dev *ad7606_probe(struct device *dev, int irq,
-			      void __iomem *base_address,
-			      const char *name, unsigned int id,
-			      const struct ad7606_bus_ops *bops);
+int ad7606_probe(struct device *dev, int irq, void __iomem *base_address,
+		 const char *name, unsigned int id,
+		 const struct ad7606_bus_ops *bops);
 int ad7606_remove(struct iio_dev *indio_dev, int irq);
 int ad7606_reset(struct ad7606_state *st);
 int ad7606_read_samples(struct ad7606_state *st);
diff --git a/drivers/staging/iio/adc/ad7606_core.c b/drivers/staging/iio/adc/ad7606_core.c
index 13d8090..1ee7a29 100644
--- a/drivers/staging/iio/adc/ad7606_core.c
+++ b/drivers/staging/iio/adc/ad7606_core.c
@@ -443,10 +443,9 @@ static const struct iio_info ad7606_info_range = {
 	.attrs = &ad7606_attribute_group_range,
 };
 
-struct iio_dev *ad7606_probe(struct device *dev, int irq,
-			     void __iomem *base_address,
-			     const char *name, unsigned int id,
-			     const struct ad7606_bus_ops *bops)
+int ad7606_probe(struct device *dev, int irq, void __iomem *base_address,
+		 const char *name, unsigned int id,
+		 const struct ad7606_bus_ops *bops)
 {
 	struct ad7606_platform_data *pdata = dev->platform_data;
 	struct ad7606_state *st;
@@ -455,7 +454,7 @@ struct iio_dev *ad7606_probe(struct device *dev, int irq,
 
 	indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
 	if (!indio_dev)
-		return ERR_PTR(-ENOMEM);
+		return -ENOMEM;
 
 	st = iio_priv(indio_dev);
 
@@ -469,7 +468,7 @@ struct iio_dev *ad7606_probe(struct device *dev, int irq,
 	if (!IS_ERR(st->reg)) {
 		ret = regulator_enable(st->reg);
 		if (ret)
-			return ERR_PTR(ret);
+			return ret;
 	}
 
 	st->pdata = pdata;
@@ -519,7 +518,7 @@ struct iio_dev *ad7606_probe(struct device *dev, int irq,
 
 	dev_set_drvdata(dev, indio_dev);
 
-	return indio_dev;
+	return 0;
 error_unregister_ring:
 	ad7606_ring_cleanup(indio_dev);
 
@@ -532,7 +531,7 @@ error_free_gpios:
 error_disable_reg:
 	if (!IS_ERR(st->reg))
 		regulator_disable(st->reg);
-	return ERR_PTR(ret);
+	return ret;
 }
 EXPORT_SYMBOL_GPL(ad7606_probe);
 
diff --git a/drivers/staging/iio/adc/ad7606_par.c b/drivers/staging/iio/adc/ad7606_par.c
index c273993..42eb9e0 100644
--- a/drivers/staging/iio/adc/ad7606_par.c
+++ b/drivers/staging/iio/adc/ad7606_par.c
@@ -51,7 +51,6 @@ static int ad7606_par_probe(struct platform_device *pdev)
 {
 	const struct platform_device_id *id = platform_get_device_id(pdev);
 	struct resource *res;
-	struct iio_dev *indio_dev;
 	void __iomem *addr;
 	resource_size_t remap_size;
 	int irq;
@@ -69,15 +68,10 @@ static int ad7606_par_probe(struct platform_device *pdev)
 
 	remap_size = resource_size(res);
 
-	indio_dev = ad7606_probe(&pdev->dev, irq, addr,
-				 id->name, id->driver_data,
-				 remap_size > 1 ? &ad7606_par16_bops :
-				 &ad7606_par8_bops);
-
-	if (IS_ERR(indio_dev))
-		return PTR_ERR(indio_dev);
-
-	return 0;
+	return ad7606_probe(&pdev->dev, irq, addr,
+			    id->name, id->driver_data,
+			    remap_size > 1 ? &ad7606_par16_bops :
+			    &ad7606_par8_bops);
 }
 
 static int ad7606_par_remove(struct platform_device *pdev)
diff --git a/drivers/staging/iio/adc/ad7606_spi.c b/drivers/staging/iio/adc/ad7606_spi.c
index 9abbc15..064c4c2 100644
--- a/drivers/staging/iio/adc/ad7606_spi.c
+++ b/drivers/staging/iio/adc/ad7606_spi.c
@@ -43,16 +43,10 @@ static const struct ad7606_bus_ops ad7606_spi_bops = {
 static int ad7606_spi_probe(struct spi_device *spi)
 {
 	const struct spi_device_id *id = spi_get_device_id(spi);
-	struct iio_dev *indio_dev;
 
-	indio_dev = ad7606_probe(&spi->dev, spi->irq, NULL,
-				 id->name, id->driver_data,
-				 &ad7606_spi_bops);
-
-	if (IS_ERR(indio_dev))
-		return PTR_ERR(indio_dev);
-
-	return 0;
+	return ad7606_probe(&spi->dev, spi->irq, NULL,
+			    id->name, id->driver_data,
+			    &ad7606_spi_bops);
 }
 
 static int ad7606_spi_remove(struct spi_device *spi)
-- 
2.1.4

[PATCH 10/13] staging:iio:ad7606: Let common remove function take a struct device *

[Lars-Peter Clausen]

Currently the common remove function takes a struct iio_dev *. This
parameter is retrieved by the individual driver remove functions by calling
get_drvdata() on their device. To simplify the code let the common remove
function directly take a struct dev * and do the IIO device in retrieval
the common remove function.

This also aligns the interface with the common probe function.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/staging/iio/adc/ad7606.h      | 2 +-
 drivers/staging/iio/adc/ad7606_core.c | 3 ++-
 drivers/staging/iio/adc/ad7606_par.c  | 6 +-----
 drivers/staging/iio/adc/ad7606_spi.c  | 4 +---
 4 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/iio/adc/ad7606.h b/drivers/staging/iio/adc/ad7606.h
index b4b25a2..6dde987 100644
--- a/drivers/staging/iio/adc/ad7606.h
+++ b/drivers/staging/iio/adc/ad7606.h
@@ -81,7 +81,7 @@ struct ad7606_bus_ops {
 int ad7606_probe(struct device *dev, int irq, void __iomem *base_address,
 		 const char *name, unsigned int id,
 		 const struct ad7606_bus_ops *bops);
-int ad7606_remove(struct iio_dev *indio_dev, int irq);
+int ad7606_remove(struct device *dev, int irq);
 int ad7606_reset(struct ad7606_state *st);
 int ad7606_read_samples(struct ad7606_state *st);
 
diff --git a/drivers/staging/iio/adc/ad7606_core.c b/drivers/staging/iio/adc/ad7606_core.c
index 1ee7a29..670caa2 100644
--- a/drivers/staging/iio/adc/ad7606_core.c
+++ b/drivers/staging/iio/adc/ad7606_core.c
@@ -535,8 +535,9 @@ error_disable_reg:
 }
 EXPORT_SYMBOL_GPL(ad7606_probe);
 
-int ad7606_remove(struct iio_dev *indio_dev, int irq)
+int ad7606_remove(struct device *dev, int irq)
 {
+	struct iio_dev *indio_dev = dev_get_drvdata(dev);
 	struct ad7606_state *st = iio_priv(indio_dev);
 
 	iio_device_unregister(indio_dev);
diff --git a/drivers/staging/iio/adc/ad7606_par.c b/drivers/staging/iio/adc/ad7606_par.c
index 42eb9e0..cd6c410c 100644
--- a/drivers/staging/iio/adc/ad7606_par.c
+++ b/drivers/staging/iio/adc/ad7606_par.c
@@ -76,11 +76,7 @@ static int ad7606_par_probe(struct platform_device *pdev)
 
 static int ad7606_par_remove(struct platform_device *pdev)
 {
-	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
-
-	ad7606_remove(indio_dev, platform_get_irq(pdev, 0));
-
-	return 0;
+	return ad7606_remove(&pdev->dev, platform_get_irq(pdev, 0));
 }
 
 static const struct platform_device_id ad7606_driver_ids[] = {
diff --git a/drivers/staging/iio/adc/ad7606_spi.c b/drivers/staging/iio/adc/ad7606_spi.c
index 064c4c2..c9b1f266 100644
--- a/drivers/staging/iio/adc/ad7606_spi.c
+++ b/drivers/staging/iio/adc/ad7606_spi.c
@@ -51,9 +51,7 @@ static int ad7606_spi_probe(struct spi_device *spi)
 
 static int ad7606_spi_remove(struct spi_device *spi)
 {
-	struct iio_dev *indio_dev = dev_get_drvdata(&spi->dev);
-
-	return ad7606_remove(indio_dev, spi->irq);
+	return ad7606_remove(&spi->dev, spi->irq);
 }
 
 static const struct spi_device_id ad7606_id[] = {
-- 
2.1.4

[PATCH 11/13] staging:iio:ad7606: Run trigger handler only once per trigger event

[Lars-Peter Clausen]

Currently the ad7606 driver installs the same function for the hard-irq and
threaded trigger handlers. This was introduced in commit 1caf7cb46135
("staging:iio:adc:ad7606 Convert to new channel registration method Update
Add missing call to iio_trigger_notify_done() Set pollfunc top and bottom
half handler"). Unfortunately the commit message does not mention why this
was done and Michael does not remember either.

Since the trigger handler function is idempotent (set a GPIO to 1) running
it twice does not do any harm, but is simply not necessary either. So set
the threaded trigger handler for the driver to NULL.

While we are at it also remove the function description comment that does
no say anything that can't be derived from the function name itself.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/staging/iio/adc/ad7606_ring.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/iio/adc/ad7606_ring.c b/drivers/staging/iio/adc/ad7606_ring.c
index b7bf0cf..81e4a0ac 100644
--- a/drivers/staging/iio/adc/ad7606_ring.c
+++ b/drivers/staging/iio/adc/ad7606_ring.c
@@ -18,11 +18,7 @@
 
 #include "ad7606.h"
 
-/**
- * ad7606_trigger_handler_th() th/bh of trigger launched polling to ring buffer
- *
- **/
-static irqreturn_t ad7606_trigger_handler_th_bh(int irq, void *p)
+static irqreturn_t ad7606_trigger_handler(int irq, void *p)
 {
 	struct iio_poll_func *pf = p;
 	struct ad7606_state *st = iio_priv(pf->indio_dev);
@@ -63,9 +59,8 @@ int ad7606_register_ring_funcs_and_init(struct iio_dev *indio_dev)
 
 	INIT_WORK(&st->poll_work, &ad7606_poll_bh_to_ring);
 
-	return iio_triggered_buffer_setup(indio_dev,
-		&ad7606_trigger_handler_th_bh, &ad7606_trigger_handler_th_bh,
-		NULL);
+	return iio_triggered_buffer_setup(indio_dev, &ad7606_trigger_handler,
+					  NULL, NULL);
 }
 
 void ad7606_ring_cleanup(struct iio_dev *indio_dev)
-- 
2.1.4

[PATCH 12/13] staging:iio:ad7606: Use GPIO descriptor API

[Lars-Peter Clausen]

Convert the ad7606 driver away from the deprecated legacy GPIO API and use
the new GPIO descriptor API.

This also means that the platform data struct is now empty and can be
removed.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/staging/iio/adc/ad7606.h      |  35 ++----
 drivers/staging/iio/adc/ad7606_core.c | 200 ++++++++++------------------------
 drivers/staging/iio/adc/ad7606_ring.c |   4 +-
 3 files changed, 64 insertions(+), 175 deletions(-)

diff --git a/drivers/staging/iio/adc/ad7606.h b/drivers/staging/iio/adc/ad7606.h
index 6dde987..f5f85fa 100644
--- a/drivers/staging/iio/adc/ad7606.h
+++ b/drivers/staging/iio/adc/ad7606.h
@@ -9,33 +9,6 @@
 #ifndef IIO_ADC_AD7606_H_
 #define IIO_ADC_AD7606_H_
 
-/*
- * TODO: struct ad7606_platform_data needs to go into include/linux/iio
- */
-
-/**
- * struct ad7606_platform_data - platform/board specific information
- * @gpio_convst:	number of gpio connected to the CONVST pin
- * @gpio_reset:		gpio connected to the RESET pin, if not used set to -1
- * @gpio_range:		gpio connected to the RANGE pin, if not used set to -1
- * @gpio_os0:		gpio connected to the OS0 pin, if not used set to -1
- * @gpio_os1:		gpio connected to the OS1 pin, if not used set to -1
- * @gpio_os2:		gpio connected to the OS2 pin, if not used set to -1
- * @gpio_frstdata:	gpio connected to the FRSTDAT pin, if not used set to -1
- * @gpio_stby:		gpio connected to the STBY pin, if not used set to -1
- */
-
-struct ad7606_platform_data {
-	unsigned int			gpio_convst;
-	unsigned int			gpio_reset;
-	unsigned int			gpio_range;
-	unsigned int			gpio_os0;
-	unsigned int			gpio_os1;
-	unsigned int			gpio_os2;
-	unsigned int			gpio_frstdata;
-	unsigned int			gpio_stby;
-};
-
 /**
  * struct ad7606_chip_info - chip specific information
  * @name:		identification string for chip
@@ -55,7 +28,6 @@ struct ad7606_chip_info {
 struct ad7606_state {
 	struct device			*dev;
 	const struct ad7606_chip_info	*chip_info;
-	struct ad7606_platform_data	*pdata;
 	struct regulator		*reg;
 	struct work_struct		poll_work;
 	wait_queue_head_t		wq_data_avail;
@@ -65,6 +37,13 @@ struct ad7606_state {
 	bool				done;
 	void __iomem			*base_address;
 
+	struct gpio_desc		*gpio_convst;
+	struct gpio_desc		*gpio_reset;
+	struct gpio_desc		*gpio_range;
+	struct gpio_desc		*gpio_standby;
+	struct gpio_desc		*gpio_frstdata;
+	struct gpio_descs		*gpio_os;
+
 	/*
 	 * DMA (thus cache coherency maintenance) requires the
 	 * transfer buffers to live in their own cache lines.
diff --git a/drivers/staging/iio/adc/ad7606_core.c b/drivers/staging/iio/adc/ad7606_core.c
index 670caa2..4ef6cb4 100644
--- a/drivers/staging/iio/adc/ad7606_core.c
+++ b/drivers/staging/iio/adc/ad7606_core.c
@@ -13,7 +13,7 @@
 #include <linux/sysfs.h>
 #include <linux/regulator/consumer.h>
 #include <linux/err.h>
-#include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
 #include <linux/delay.h>
 #include <linux/sched.h>
 #include <linux/module.h>
@@ -26,10 +26,10 @@
 
 int ad7606_reset(struct ad7606_state *st)
 {
-	if (gpio_is_valid(st->pdata->gpio_reset)) {
-		gpio_set_value(st->pdata->gpio_reset, 1);
+	if (st->gpio_reset) {
+		gpiod_set_value(st->gpio_reset, 1);
 		ndelay(100); /* t_reset >= 100ns */
-		gpio_set_value(st->pdata->gpio_reset, 0);
+		gpiod_set_value(st->gpio_reset, 0);
 		return 0;
 	}
 
@@ -52,12 +52,12 @@ int ad7606_read_samples(struct ad7606_state *st)
 	 * situations.
 	 */
 
-	if (gpio_is_valid(st->pdata->gpio_frstdata)) {
+	if (st->gpio_frstdata) {
 		ret = st->bops->read_block(st->dev, 1, data);
 		if (ret)
 			return ret;
 
-		if (!gpio_get_value(st->pdata->gpio_frstdata)) {
+		if (!gpiod_get_value(st->gpio_frstdata)) {
 			ad7606_reset(st);
 			return -EIO;
 		}
@@ -75,7 +75,7 @@ static int ad7606_scan_direct(struct iio_dev *indio_dev, unsigned int ch)
 	int ret;
 
 	st->done = false;
-	gpio_set_value(st->pdata->gpio_convst, 1);
+	gpiod_set_value(st->gpio_convst, 1);
 
 	ret = wait_event_interruptible(st->wq_data_avail, st->done);
 	if (ret)
@@ -86,7 +86,7 @@ static int ad7606_scan_direct(struct iio_dev *indio_dev, unsigned int ch)
 		ret = st->data[ch];
 
 error_ret:
-	gpio_set_value(st->pdata->gpio_convst, 0);
+	gpiod_set_value(st->gpio_convst, 0);
 
 	return ret;
 }
@@ -150,7 +150,7 @@ static ssize_t ad7606_store_range(struct device *dev,
 		return -EINVAL;
 
 	mutex_lock(&indio_dev->mlock);
-	gpio_set_value(st->pdata->gpio_range, lval == 10000);
+	gpiod_set_value(st->gpio_range, lval == 10000);
 	st->range = lval;
 	mutex_unlock(&indio_dev->mlock);
 
@@ -180,6 +180,7 @@ static int ad7606_write_raw(struct iio_dev *indio_dev,
 			    long mask)
 {
 	struct ad7606_state *st = iio_priv(indio_dev);
+	int values[3];
 	int ret;
 
 	switch (mask) {
@@ -190,12 +191,16 @@ static int ad7606_write_raw(struct iio_dev *indio_dev,
 		if (ret < 0)
 			return ret;
 
+		values[0] = (ret >> 0) & 1;
+		values[1] = (ret >> 1) & 1;
+		values[2] = (ret >> 2) & 1;
+
 		mutex_lock(&indio_dev->mlock);
-		gpio_set_value(st->pdata->gpio_os0, (ret >> 0) & 1);
-		gpio_set_value(st->pdata->gpio_os1, (ret >> 1) & 1);
-		gpio_set_value(st->pdata->gpio_os2, (ret >> 2) & 1);
+		gpiod_set_array_value(ARRAY_SIZE(values), st->gpio_os->desc,
+				      values);
 		st->oversampling = val;
 		mutex_unlock(&indio_dev->mlock);
+
 		return 0;
 	default:
 		return -EINVAL;
@@ -285,119 +290,37 @@ static const struct ad7606_chip_info ad7606_chip_info_tbl[] = {
 
 static int ad7606_request_gpios(struct ad7606_state *st)
 {
-	struct gpio gpio_array[3] = {
-		[0] = {
-			.gpio =  st->pdata->gpio_os0,
-			.flags = GPIOF_DIR_OUT | ((st->oversampling & 1) ?
-				 GPIOF_INIT_HIGH : GPIOF_INIT_LOW),
-			.label = "AD7606_OS0",
-		},
-		[1] = {
-			.gpio =  st->pdata->gpio_os1,
-			.flags = GPIOF_DIR_OUT | ((st->oversampling & 2) ?
-				 GPIOF_INIT_HIGH : GPIOF_INIT_LOW),
-			.label = "AD7606_OS1",
-		},
-		[2] = {
-			.gpio =  st->pdata->gpio_os2,
-			.flags = GPIOF_DIR_OUT | ((st->oversampling & 4) ?
-				 GPIOF_INIT_HIGH : GPIOF_INIT_LOW),
-			.label = "AD7606_OS2",
-		},
-	};
-	int ret;
+	struct device *dev = st->dev;
 
-	if (gpio_is_valid(st->pdata->gpio_convst)) {
-		ret = gpio_request_one(st->pdata->gpio_convst,
-				       GPIOF_OUT_INIT_LOW,
-				       "AD7606_CONVST");
-		if (ret) {
-			dev_err(st->dev, "failed to request GPIO CONVST\n");
-			goto error_ret;
-		}
-	} else {
-		ret = -EIO;
-		goto error_ret;
-	}
+	st->gpio_convst = devm_gpiod_get(dev, "conversion-start",
+					 GPIOD_OUT_LOW);
+	if (IS_ERR(st->gpio_convst))
+		return PTR_ERR(st->gpio_convst);
 
-	if (gpio_is_valid(st->pdata->gpio_os0) &&
-	    gpio_is_valid(st->pdata->gpio_os1) &&
-	    gpio_is_valid(st->pdata->gpio_os2)) {
-		ret = gpio_request_array(gpio_array, ARRAY_SIZE(gpio_array));
-		if (ret < 0)
-			goto error_free_convst;
-	}
+	st->gpio_reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
+	if (IS_ERR(st->gpio_reset))
+		return PTR_ERR(st->gpio_reset);
 
-	if (gpio_is_valid(st->pdata->gpio_reset)) {
-		ret = gpio_request_one(st->pdata->gpio_reset,
-				       GPIOF_OUT_INIT_LOW,
-				       "AD7606_RESET");
-		if (ret < 0)
-			goto error_free_os;
-	}
+	st->gpio_range = devm_gpiod_get_optional(dev, "range", GPIOD_OUT_LOW);
+	if (IS_ERR(st->gpio_range))
+		return PTR_ERR(st->gpio_range);
 
-	if (gpio_is_valid(st->pdata->gpio_range)) {
-		ret = gpio_request_one(st->pdata->gpio_range, GPIOF_DIR_OUT |
-				       ((st->range == 10000) ? GPIOF_INIT_HIGH :
-					GPIOF_INIT_LOW), "AD7606_RANGE");
-		if (ret < 0)
-			goto error_free_reset;
-	}
-	if (gpio_is_valid(st->pdata->gpio_stby)) {
-		ret = gpio_request_one(st->pdata->gpio_stby,
-				       GPIOF_OUT_INIT_HIGH,
-				       "AD7606_STBY");
-		if (ret < 0)
-			goto error_free_range;
-	}
+	st->gpio_standby = devm_gpiod_get_optional(dev, "standby",
+						   GPIOD_OUT_HIGH);
+	if (IS_ERR(st->gpio_standby))
+		return PTR_ERR(st->gpio_standby);
 
-	if (gpio_is_valid(st->pdata->gpio_frstdata)) {
-		ret = gpio_request_one(st->pdata->gpio_frstdata, GPIOF_IN,
-				       "AD7606_FRSTDATA");
-		if (ret < 0)
-			goto error_free_stby;
-	}
+	st->gpio_frstdata = devm_gpiod_get_optional(dev, "first-data",
+						    GPIOD_IN);
+	if (IS_ERR(st->gpio_frstdata))
+		return PTR_ERR(st->gpio_frstdata);
 
-	return 0;
+	st->gpio_os = devm_gpiod_get_array_optional(dev, "oversampling-ratio",
+			GPIOD_OUT_LOW);
+	if (IS_ERR(st->gpio_os))
+		return PTR_ERR(st->gpio_os);
 
-error_free_stby:
-	if (gpio_is_valid(st->pdata->gpio_stby))
-		gpio_free(st->pdata->gpio_stby);
-error_free_range:
-	if (gpio_is_valid(st->pdata->gpio_range))
-		gpio_free(st->pdata->gpio_range);
-error_free_reset:
-	if (gpio_is_valid(st->pdata->gpio_reset))
-		gpio_free(st->pdata->gpio_reset);
-error_free_os:
-	if (gpio_is_valid(st->pdata->gpio_os0) &&
-	    gpio_is_valid(st->pdata->gpio_os1) &&
-	    gpio_is_valid(st->pdata->gpio_os2))
-		gpio_free_array(gpio_array, ARRAY_SIZE(gpio_array));
-error_free_convst:
-	gpio_free(st->pdata->gpio_convst);
-error_ret:
-	return ret;
-}
-
-static void ad7606_free_gpios(struct ad7606_state *st)
-{
-	if (gpio_is_valid(st->pdata->gpio_frstdata))
-		gpio_free(st->pdata->gpio_frstdata);
-	if (gpio_is_valid(st->pdata->gpio_stby))
-		gpio_free(st->pdata->gpio_stby);
-	if (gpio_is_valid(st->pdata->gpio_range))
-		gpio_free(st->pdata->gpio_range);
-	if (gpio_is_valid(st->pdata->gpio_reset))
-		gpio_free(st->pdata->gpio_reset);
-	if (gpio_is_valid(st->pdata->gpio_os0) &&
-	    gpio_is_valid(st->pdata->gpio_os1) &&
-	    gpio_is_valid(st->pdata->gpio_os2)) {
-		gpio_free(st->pdata->gpio_os2);
-		gpio_free(st->pdata->gpio_os1);
-		gpio_free(st->pdata->gpio_os0);
-	}
-	gpio_free(st->pdata->gpio_convst);
+	return 0;
 }
 
 /**
@@ -447,7 +370,6 @@ int ad7606_probe(struct device *dev, int irq, void __iomem *base_address,
 		 const char *name, unsigned int id,
 		 const struct ad7606_bus_ops *bops)
 {
-	struct ad7606_platform_data *pdata = dev->platform_data;
 	struct ad7606_state *st;
 	int ret;
 	struct iio_dev *indio_dev;
@@ -471,19 +393,20 @@ int ad7606_probe(struct device *dev, int irq, void __iomem *base_address,
 			return ret;
 	}
 
-	st->pdata = pdata;
+	ret = ad7606_request_gpios(st);
+	if (ret)
+		goto error_disable_reg;
+
 	st->chip_info = &ad7606_chip_info_tbl[id];
 
 	indio_dev->dev.parent = dev;
-	if (gpio_is_valid(st->pdata->gpio_os0) &&
-	    gpio_is_valid(st->pdata->gpio_os1) &&
-	    gpio_is_valid(st->pdata->gpio_os2)) {
-		if (gpio_is_valid(st->pdata->gpio_range))
+	if (st->gpio_os) {
+		if (st->gpio_range)
 			indio_dev->info = &ad7606_info_os_and_range;
 		else
 			indio_dev->info = &ad7606_info_os;
 	} else {
-		if (gpio_is_valid(st->pdata->gpio_range))
+		if (st->gpio_range)
 			indio_dev->info = &ad7606_info_range;
 		else
 			indio_dev->info = &ad7606_info_no_os_or_range;
@@ -495,10 +418,6 @@ int ad7606_probe(struct device *dev, int irq, void __iomem *base_address,
 
 	init_waitqueue_head(&st->wq_data_avail);
 
-	ret = ad7606_request_gpios(st);
-	if (ret)
-		goto error_disable_reg;
-
 	ret = ad7606_reset(st);
 	if (ret)
 		dev_warn(st->dev, "failed to RESET: no RESET GPIO specified\n");
@@ -506,7 +425,7 @@ int ad7606_probe(struct device *dev, int irq, void __iomem *base_address,
 	ret = request_irq(irq, ad7606_interrupt, IRQF_TRIGGER_FALLING, name,
 			  indio_dev);
 	if (ret)
-		goto error_free_gpios;
+		goto error_disable_reg;
 
 	ret = ad7606_register_ring_funcs_and_init(indio_dev);
 	if (ret)
@@ -525,9 +444,6 @@ error_unregister_ring:
 error_free_irq:
 	free_irq(irq, indio_dev);
 
-error_free_gpios:
-	ad7606_free_gpios(st);
-
 error_disable_reg:
 	if (!IS_ERR(st->reg))
 		regulator_disable(st->reg);
@@ -547,8 +463,6 @@ int ad7606_remove(struct device *dev, int irq)
 	if (!IS_ERR(st->reg))
 		regulator_disable(st->reg);
 
-	ad7606_free_gpios(st);
-
 	return 0;
 }
 EXPORT_SYMBOL_GPL(ad7606_remove);
@@ -560,10 +474,9 @@ static int ad7606_suspend(struct device *dev)
 	struct iio_dev *indio_dev = dev_get_drvdata(dev);
 	struct ad7606_state *st = iio_priv(indio_dev);
 
-	if (gpio_is_valid(st->pdata->gpio_stby)) {
-		if (gpio_is_valid(st->pdata->gpio_range))
-			gpio_set_value(st->pdata->gpio_range, 1);
-		gpio_set_value(st->pdata->gpio_stby, 0);
+	if (st->gpio_standby) {
+		gpiod_set_value(st->gpio_range, 1);
+		gpiod_set_value(st->gpio_standby, 0);
 	}
 
 	return 0;
@@ -574,12 +487,9 @@ static int ad7606_resume(struct device *dev)
 	struct iio_dev *indio_dev = dev_get_drvdata(dev);
 	struct ad7606_state *st = iio_priv(indio_dev);
 
-	if (gpio_is_valid(st->pdata->gpio_stby)) {
-		if (gpio_is_valid(st->pdata->gpio_range))
-			gpio_set_value(st->pdata->gpio_range,
-				       st->range == 10000);
-
-		gpio_set_value(st->pdata->gpio_stby, 1);
+	if (st->gpio_standby) {
+		gpiod_set_value(st->gpio_range, st->range == 10000);
+		gpiod_set_value(st->gpio_standby, 1);
 		ad7606_reset(st);
 	}
 
diff --git a/drivers/staging/iio/adc/ad7606_ring.c b/drivers/staging/iio/adc/ad7606_ring.c
index 81e4a0ac..68d72b1 100644
--- a/drivers/staging/iio/adc/ad7606_ring.c
+++ b/drivers/staging/iio/adc/ad7606_ring.c
@@ -23,7 +23,7 @@ static irqreturn_t ad7606_trigger_handler(int irq, void *p)
 	struct iio_poll_func *pf = p;
 	struct ad7606_state *st = iio_priv(pf->indio_dev);
 
-	gpio_set_value(st->pdata->gpio_convst, 1);
+	gpiod_set_value(st->gpio_convst, 1);
 
 	return IRQ_HANDLED;
 }
@@ -49,7 +49,7 @@ static void ad7606_poll_bh_to_ring(struct work_struct *work_s)
 		iio_push_to_buffers_with_timestamp(indio_dev, st->data,
 						   iio_get_time_ns(indio_dev));
 
-	gpio_set_value(st->pdata->gpio_convst, 0);
+	gpiod_set_value(st->gpio_convst, 0);
 	iio_trigger_notify_done(indio_dev->trig);
 }
 
-- 
2.1.4

[PATCH 13/13] staging:iio:ad7606: Move buffer code to main source file

[Lars-Peter Clausen]

Currently the ad7606 buffer handling code resides in its own source file.
But this file contains only 4 small functions of which half are just
wrappers around other functions. Buffer support is also always enabled for
this driver, so move them over to the main source file. This reduces the
amount of boilerplate code.

Also rename the main function from ad7606_core.c to ad7606.c since there is
only a single file now.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/staging/iio/adc/Makefile                   |  1 -
 .../staging/iio/adc/{ad7606_core.c => ad7606.c}    | 49 +++++++++++++--
 drivers/staging/iio/adc/ad7606.h                   |  5 --
 drivers/staging/iio/adc/ad7606_ring.c              | 69 ----------------------
 4 files changed, 44 insertions(+), 80 deletions(-)
 rename drivers/staging/iio/adc/{ad7606_core.c => ad7606.c} (88%)
 delete mode 100644 drivers/staging/iio/adc/ad7606_ring.c

diff --git a/drivers/staging/iio/adc/Makefile b/drivers/staging/iio/adc/Makefile
index 3cdd83c..ac09485 100644
--- a/drivers/staging/iio/adc/Makefile
+++ b/drivers/staging/iio/adc/Makefile
@@ -2,7 +2,6 @@
 # Makefile for industrial I/O ADC drivers
 #
 
-ad7606-y := ad7606_core.o ad7606_ring.o
 obj-$(CONFIG_AD7606_IFACE_PARALLEL) += ad7606_par.o
 obj-$(CONFIG_AD7606_IFACE_SPI) += ad7606_spi.o
 obj-$(CONFIG_AD7606) += ad7606.o
diff --git a/drivers/staging/iio/adc/ad7606_core.c b/drivers/staging/iio/adc/ad7606.c
similarity index 88%
rename from drivers/staging/iio/adc/ad7606_core.c
rename to drivers/staging/iio/adc/ad7606.c
index 4ef6cb4..010c6e1 100644
--- a/drivers/staging/iio/adc/ad7606_core.c
+++ b/drivers/staging/iio/adc/ad7606.c
@@ -21,10 +21,12 @@
 #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 "ad7606.h"
 
-int ad7606_reset(struct ad7606_state *st)
+static int ad7606_reset(struct ad7606_state *st)
 {
 	if (st->gpio_reset) {
 		gpiod_set_value(st->gpio_reset, 1);
@@ -36,7 +38,7 @@ int ad7606_reset(struct ad7606_state *st)
 	return -ENODEV;
 }
 
-int ad7606_read_samples(struct ad7606_state *st)
+static int ad7606_read_samples(struct ad7606_state *st)
 {
 	unsigned int num = st->chip_info->num_channels;
 	u16 *data = st->data;
@@ -69,6 +71,41 @@ int ad7606_read_samples(struct ad7606_state *st)
 	return st->bops->read_block(st->dev, num, data);
 }
 
+static irqreturn_t ad7606_trigger_handler(int irq, void *p)
+{
+	struct iio_poll_func *pf = p;
+	struct ad7606_state *st = iio_priv(pf->indio_dev);
+
+	gpiod_set_value(st->gpio_convst, 1);
+
+	return IRQ_HANDLED;
+}
+
+/**
+ * ad7606_poll_bh_to_ring() bh of trigger launched polling to ring buffer
+ * @work_s:	the work struct through which this was scheduled
+ *
+ * Currently there is no option in this driver to disable the saving of
+ * timestamps within the ring.
+ * I think the one copy of this at a time was to avoid problems if the
+ * trigger was set far too high and the reads then locked up the computer.
+ **/
+static void ad7606_poll_bh_to_ring(struct work_struct *work_s)
+{
+	struct ad7606_state *st = container_of(work_s, struct ad7606_state,
+						poll_work);
+	struct iio_dev *indio_dev = iio_priv_to_dev(st);
+	int ret;
+
+	ret = ad7606_read_samples(st);
+	if (ret == 0)
+		iio_push_to_buffers_with_timestamp(indio_dev, st->data,
+						   iio_get_time_ns(indio_dev));
+
+	gpiod_set_value(st->gpio_convst, 0);
+	iio_trigger_notify_done(indio_dev->trig);
+}
+
 static int ad7606_scan_direct(struct iio_dev *indio_dev, unsigned int ch)
 {
 	struct ad7606_state *st = iio_priv(indio_dev);
@@ -385,6 +422,7 @@ int ad7606_probe(struct device *dev, int irq, void __iomem *base_address,
 	st->base_address = base_address;
 	st->range = 5000;
 	st->oversampling = 1;
+	INIT_WORK(&st->poll_work, &ad7606_poll_bh_to_ring);
 
 	st->reg = devm_regulator_get(dev, "vcc");
 	if (!IS_ERR(st->reg)) {
@@ -427,7 +465,8 @@ int ad7606_probe(struct device *dev, int irq, void __iomem *base_address,
 	if (ret)
 		goto error_disable_reg;
 
-	ret = ad7606_register_ring_funcs_and_init(indio_dev);
+	ret = iio_triggered_buffer_setup(indio_dev, &ad7606_trigger_handler,
+					 NULL, NULL);
 	if (ret)
 		goto error_free_irq;
 
@@ -439,7 +478,7 @@ int ad7606_probe(struct device *dev, int irq, void __iomem *base_address,
 
 	return 0;
 error_unregister_ring:
-	ad7606_ring_cleanup(indio_dev);
+	iio_triggered_buffer_cleanup(indio_dev);
 
 error_free_irq:
 	free_irq(irq, indio_dev);
@@ -457,7 +496,7 @@ int ad7606_remove(struct device *dev, int irq)
 	struct ad7606_state *st = iio_priv(indio_dev);
 
 	iio_device_unregister(indio_dev);
-	ad7606_ring_cleanup(indio_dev);
+	iio_triggered_buffer_cleanup(indio_dev);
 
 	free_irq(irq, indio_dev);
 	if (!IS_ERR(st->reg))
diff --git a/drivers/staging/iio/adc/ad7606.h b/drivers/staging/iio/adc/ad7606.h
index f5f85fa..746f955 100644
--- a/drivers/staging/iio/adc/ad7606.h
+++ b/drivers/staging/iio/adc/ad7606.h
@@ -61,8 +61,6 @@ int ad7606_probe(struct device *dev, int irq, void __iomem *base_address,
 		 const char *name, unsigned int id,
 		 const struct ad7606_bus_ops *bops);
 int ad7606_remove(struct device *dev, int irq);
-int ad7606_reset(struct ad7606_state *st);
-int ad7606_read_samples(struct ad7606_state *st);
 
 enum ad7606_supported_device_ids {
 	ID_AD7606_8,
@@ -70,9 +68,6 @@ enum ad7606_supported_device_ids {
 	ID_AD7606_4
 };
 
-int ad7606_register_ring_funcs_and_init(struct iio_dev *indio_dev);
-void ad7606_ring_cleanup(struct iio_dev *indio_dev);
-
 #ifdef CONFIG_PM_SLEEP
 extern const struct dev_pm_ops ad7606_pm_ops;
 #define AD7606_PM_OPS (&ad7606_pm_ops)
diff --git a/drivers/staging/iio/adc/ad7606_ring.c b/drivers/staging/iio/adc/ad7606_ring.c
deleted file mode 100644
index 68d72b1..0000000
--- a/drivers/staging/iio/adc/ad7606_ring.c
+++ /dev/null
@@ -1,69 +0,0 @@
-/*
- * Copyright 2011-2012 Analog Devices Inc.
- *
- * Licensed under the GPL-2.
- *
- */
-
-#include <linux/interrupt.h>
-#include <linux/gpio.h>
-#include <linux/device.h>
-#include <linux/kernel.h>
-#include <linux/slab.h>
-
-#include <linux/iio/iio.h>
-#include <linux/iio/buffer.h>
-#include <linux/iio/trigger_consumer.h>
-#include <linux/iio/triggered_buffer.h>
-
-#include "ad7606.h"
-
-static irqreturn_t ad7606_trigger_handler(int irq, void *p)
-{
-	struct iio_poll_func *pf = p;
-	struct ad7606_state *st = iio_priv(pf->indio_dev);
-
-	gpiod_set_value(st->gpio_convst, 1);
-
-	return IRQ_HANDLED;
-}
-
-/**
- * ad7606_poll_bh_to_ring() bh of trigger launched polling to ring buffer
- * @work_s:	the work struct through which this was scheduled
- *
- * Currently there is no option in this driver to disable the saving of
- * timestamps within the ring.
- * I think the one copy of this at a time was to avoid problems if the
- * trigger was set far too high and the reads then locked up the computer.
- **/
-static void ad7606_poll_bh_to_ring(struct work_struct *work_s)
-{
-	struct ad7606_state *st = container_of(work_s, struct ad7606_state,
-						poll_work);
-	struct iio_dev *indio_dev = iio_priv_to_dev(st);
-	int ret;
-
-	ret = ad7606_read_samples(st);
-	if (ret == 0)
-		iio_push_to_buffers_with_timestamp(indio_dev, st->data,
-						   iio_get_time_ns(indio_dev));
-
-	gpiod_set_value(st->gpio_convst, 0);
-	iio_trigger_notify_done(indio_dev->trig);
-}
-
-int ad7606_register_ring_funcs_and_init(struct iio_dev *indio_dev)
-{
-	struct ad7606_state *st = iio_priv(indio_dev);
-
-	INIT_WORK(&st->poll_work, &ad7606_poll_bh_to_ring);
-
-	return iio_triggered_buffer_setup(indio_dev, &ad7606_trigger_handler,
-					  NULL, NULL);
-}
-
-void ad7606_ring_cleanup(struct iio_dev *indio_dev)
-{
-	iio_triggered_buffer_cleanup(indio_dev);
-}
-- 
2.1.4

Re: [PATCH 01/13] staging:iio:ad7606: Remove unused int_vref_mv field

[Jonathan Cameron]

On 19/10/16 18:06, Lars-Peter Clausen wrote:
> Remove the int_vref_mv field from the ad7606_chip_info struct since the
> field is never used by the driver. The value is also the same for all
> derivatives of this chip, so if it will ever be used in the driver a
> constant value will work just fine.
> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Applied to the togreg branch of iio.git - initially pushed out as
testing for the autobuilders to play with it.

Thanks,

Jonathan
> ---
>  drivers/staging/iio/adc/ad7606.h      | 2 --
>  drivers/staging/iio/adc/ad7606_core.c | 3 ---
>  2 files changed, 5 deletions(-)
> 
> diff --git a/drivers/staging/iio/adc/ad7606.h b/drivers/staging/iio/adc/ad7606.h
> index 39f5044..b7e59a6 100644
> --- a/drivers/staging/iio/adc/ad7606.h
> +++ b/drivers/staging/iio/adc/ad7606.h
> @@ -43,14 +43,12 @@ struct ad7606_platform_data {
>  /**
>   * struct ad7606_chip_info - chip specific information
>   * @name:		identification string for chip
> - * @int_vref_mv:	the internal reference voltage
>   * @channels:		channel specification
>   * @num_channels:	number of channels
>   */
>  
>  struct ad7606_chip_info {
>  	const char			*name;
> -	u16				int_vref_mv;
>  	const struct iio_chan_spec	*channels;
>  	unsigned int			num_channels;
>  };
> diff --git a/drivers/staging/iio/adc/ad7606_core.c b/drivers/staging/iio/adc/ad7606_core.c
> index 2042225..a1f18d4 100644
> --- a/drivers/staging/iio/adc/ad7606_core.c
> +++ b/drivers/staging/iio/adc/ad7606_core.c
> @@ -261,19 +261,16 @@ static const struct ad7606_chip_info ad7606_chip_info_tbl[] = {
>  	 */
>  	[ID_AD7606_8] = {
>  		.name = "ad7606",
> -		.int_vref_mv = 2500,
>  		.channels = ad7606_channels,
>  		.num_channels = 9,
>  	},
>  	[ID_AD7606_6] = {
>  		.name = "ad7606-6",
> -		.int_vref_mv = 2500,
>  		.channels = ad7606_channels,
>  		.num_channels = 7,
>  	},
>  	[ID_AD7606_4] = {
>  		.name = "ad7606-4",
> -		.int_vref_mv = 2500,
>  		.channels = ad7606_channels,
>  		.num_channels = 5,
>  	},
> 

Re: [PATCH 02/13] staging:iio:ad7606: Remove redundant name field from ad7606_chip_info

[Jonathan Cameron]

On 19/10/16 18:06, Lars-Peter Clausen wrote:
> The name field in the ad7606_chip_info struct is set to the same value as
> the as the name field in the corresponding {platform,spi}_device_id table
> entry. Remove it from the ad7606_chip_info struct and pass the name from
> the ID to the probe function. This slightly reduces the size of the
> chip_info table and adding new entries requires less boilerplate.
> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Applied.
> ---
>  drivers/staging/iio/adc/ad7606.h      |  4 ++--
>  drivers/staging/iio/adc/ad7606_core.c | 11 ++++-------
>  drivers/staging/iio/adc/ad7606_par.c  |  3 ++-
>  drivers/staging/iio/adc/ad7606_spi.c  |  3 ++-
>  4 files changed, 10 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/staging/iio/adc/ad7606.h b/drivers/staging/iio/adc/ad7606.h
> index b7e59a6..380c56a 100644
> --- a/drivers/staging/iio/adc/ad7606.h
> +++ b/drivers/staging/iio/adc/ad7606.h
> @@ -48,7 +48,6 @@ struct ad7606_platform_data {
>   */
>  
>  struct ad7606_chip_info {
> -	const char			*name;
>  	const struct iio_chan_spec	*channels;
>  	unsigned int			num_channels;
>  };
> @@ -84,7 +83,8 @@ struct ad7606_bus_ops {
>  };
>  
>  struct iio_dev *ad7606_probe(struct device *dev, int irq,
> -			      void __iomem *base_address, unsigned int id,
> +			      void __iomem *base_address,
> +			      const char *name, unsigned int id,
>  			      const struct ad7606_bus_ops *bops);
>  int ad7606_remove(struct iio_dev *indio_dev, int irq);
>  int ad7606_reset(struct ad7606_state *st);
> diff --git a/drivers/staging/iio/adc/ad7606_core.c b/drivers/staging/iio/adc/ad7606_core.c
> index a1f18d4..1093647 100644
> --- a/drivers/staging/iio/adc/ad7606_core.c
> +++ b/drivers/staging/iio/adc/ad7606_core.c
> @@ -260,17 +260,14 @@ static const struct ad7606_chip_info ad7606_chip_info_tbl[] = {
>  	 * More devices added in future
>  	 */
>  	[ID_AD7606_8] = {
> -		.name = "ad7606",
>  		.channels = ad7606_channels,
>  		.num_channels = 9,
>  	},
>  	[ID_AD7606_6] = {
> -		.name = "ad7606-6",
>  		.channels = ad7606_channels,
>  		.num_channels = 7,
>  	},
>  	[ID_AD7606_4] = {
> -		.name = "ad7606-4",
>  		.channels = ad7606_channels,
>  		.num_channels = 5,
>  	},
> @@ -438,7 +435,7 @@ static const struct iio_info ad7606_info_range = {
>  
>  struct iio_dev *ad7606_probe(struct device *dev, int irq,
>  			     void __iomem *base_address,
> -			     unsigned int id,
> +			     const char *name, unsigned int id,
>  			     const struct ad7606_bus_ops *bops)
>  {
>  	struct ad7606_platform_data *pdata = dev->platform_data;
> @@ -491,7 +488,7 @@ struct iio_dev *ad7606_probe(struct device *dev, int irq,
>  			indio_dev->info = &ad7606_info_no_os_or_range;
>  	}
>  	indio_dev->modes = INDIO_DIRECT_MODE;
> -	indio_dev->name = st->chip_info->name;
> +	indio_dev->name = name;
>  	indio_dev->channels = st->chip_info->channels;
>  	indio_dev->num_channels = st->chip_info->num_channels;
>  
> @@ -505,8 +502,8 @@ struct iio_dev *ad7606_probe(struct device *dev, int irq,
>  	if (ret)
>  		dev_warn(st->dev, "failed to RESET: no RESET GPIO specified\n");
>  
> -	ret = request_irq(irq, ad7606_interrupt,
> -			  IRQF_TRIGGER_FALLING, st->chip_info->name, indio_dev);
> +	ret = request_irq(irq, ad7606_interrupt, IRQF_TRIGGER_FALLING, name,
> +			  indio_dev);
>  	if (ret)
>  		goto error_free_gpios;
>  
> diff --git a/drivers/staging/iio/adc/ad7606_par.c b/drivers/staging/iio/adc/ad7606_par.c
> index 84d2393..0a23cd9 100644
> --- a/drivers/staging/iio/adc/ad7606_par.c
> +++ b/drivers/staging/iio/adc/ad7606_par.c
> @@ -49,6 +49,7 @@ static const struct ad7606_bus_ops ad7606_par8_bops = {
>  
>  static int ad7606_par_probe(struct platform_device *pdev)
>  {
> +	const struct platform_device_id *id = platform_get_device_id(pdev);
>  	struct resource *res;
>  	struct iio_dev *indio_dev;
>  	void __iomem *addr;
> @@ -69,7 +70,7 @@ static int ad7606_par_probe(struct platform_device *pdev)
>  	remap_size = resource_size(res);
>  
>  	indio_dev = ad7606_probe(&pdev->dev, irq, addr,
> -				 platform_get_device_id(pdev)->driver_data,
> +				 id->name, id->driver_data,
>  				 remap_size > 1 ? &ad7606_par16_bops :
>  				 &ad7606_par8_bops);
>  
> diff --git a/drivers/staging/iio/adc/ad7606_spi.c b/drivers/staging/iio/adc/ad7606_spi.c
> index 9587fa8..f69db59 100644
> --- a/drivers/staging/iio/adc/ad7606_spi.c
> +++ b/drivers/staging/iio/adc/ad7606_spi.c
> @@ -42,10 +42,11 @@ static const struct ad7606_bus_ops ad7606_spi_bops = {
>  
>  static int ad7606_spi_probe(struct spi_device *spi)
>  {
> +	const struct spi_device_id *id = spi_get_device_id(spi);
>  	struct iio_dev *indio_dev;
>  
>  	indio_dev = ad7606_probe(&spi->dev, spi->irq, NULL,
> -				 spi_get_device_id(spi)->driver_data,
> +				 id->name, id->driver_data,
>  				 &ad7606_spi_bops);
>  
>  	if (IS_ERR(indio_dev))
> 

Re: [PATCH 03/13] staging:iio:ad7606: Remove default device configuration from platform data

[Jonathan Cameron]

On 19/10/16 18:06, Lars-Peter Clausen wrote:
> While for some very selected setups it might be useful to be able to
> provide default configuration data via the platform data, generally this
> becomes very impractical as the number of configuration options increases.
> So the general policy is to use the power-on default values of the device
> and let the application using the device configure it according to its
> needs.
> 
> Implement this scheme for the ad7606 driver by removing support for
> specifying a default configuration via the platform data.
> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Applied.

Thanks,

Jonathan
> ---
>  drivers/staging/iio/adc/ad7606.h      |  4 ----
>  drivers/staging/iio/adc/ad7606_core.c | 12 ++----------
>  2 files changed, 2 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/staging/iio/adc/ad7606.h b/drivers/staging/iio/adc/ad7606.h
> index 380c56a..cf6be65 100644
> --- a/drivers/staging/iio/adc/ad7606.h
> +++ b/drivers/staging/iio/adc/ad7606.h
> @@ -15,8 +15,6 @@
>  
>  /**
>   * struct ad7606_platform_data - platform/board specific information
> - * @default_os:		default oversampling value {0, 2, 4, 8, 16, 32, 64}
> - * @default_range:	default range +/-{5000, 10000} mVolt
>   * @gpio_convst:	number of gpio connected to the CONVST pin
>   * @gpio_reset:		gpio connected to the RESET pin, if not used set to -1
>   * @gpio_range:		gpio connected to the RANGE pin, if not used set to -1
> @@ -28,8 +26,6 @@
>   */
>  
>  struct ad7606_platform_data {
> -	unsigned int			default_os;
> -	unsigned int			default_range;
>  	unsigned int			gpio_convst;
>  	unsigned int			gpio_reset;
>  	unsigned int			gpio_range;
> diff --git a/drivers/staging/iio/adc/ad7606_core.c b/drivers/staging/iio/adc/ad7606_core.c
> index 1093647..a16c6f5 100644
> --- a/drivers/staging/iio/adc/ad7606_core.c
> +++ b/drivers/staging/iio/adc/ad7606_core.c
> @@ -452,16 +452,8 @@ struct iio_dev *ad7606_probe(struct device *dev, int irq,
>  	st->dev = dev;
>  	st->bops = bops;
>  	st->base_address = base_address;
> -	st->range = pdata->default_range == 10000 ? 10000 : 5000;
> -
> -	ret = ad7606_oversampling_get_index(pdata->default_os);
> -	if (ret < 0) {
> -		dev_warn(dev, "oversampling %d is not supported\n",
> -			 pdata->default_os);
> -		st->oversampling = 0;
> -	} else {
> -		st->oversampling = pdata->default_os;
> -	}
> +	st->range = 5000;
> +	st->oversampling = 0;
>  
>  	st->reg = devm_regulator_get(dev, "vcc");
>  	if (!IS_ERR(st->reg)) {
> 

Re: [PATCH 04/13] staging:iio:ad7606: Remove out-of-band error reporting

[Jonathan Cameron]

On 19/10/16 18:06, Lars-Peter Clausen wrote:
> Currently the ad7606 driver prints a error message to the kernel log when
> an application writes an invalid value to a sysfs attribute. While for
> initial driver development and testing this might be useful it is quite
> disadvantageous in a production environment. The write() call to the sysfs
> attribute will already return an error if the value was invalid so the
> application is aware that the operation failed. And generally speaking it
> is impossible for an application to reliably match a log message in the
> kernel log to a specific operation it performed, so the message becomes
> just noise and might distract from more critical messages.
> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Applied.

Thanks,

Jonathan
> ---
>  drivers/staging/iio/adc/ad7606_core.c | 10 +++-------
>  1 file changed, 3 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/staging/iio/adc/ad7606_core.c b/drivers/staging/iio/adc/ad7606_core.c
> index a16c6f5..d3a3551 100644
> --- a/drivers/staging/iio/adc/ad7606_core.c
> +++ b/drivers/staging/iio/adc/ad7606_core.c
> @@ -132,10 +132,9 @@ static ssize_t ad7606_store_range(struct device *dev,
>  	if (ret)
>  		return ret;
>  
> -	if (!(lval == 5000 || lval == 10000)) {
> -		dev_err(dev, "range is not supported\n");
> +	if (!(lval == 5000 || lval == 10000))
>  		return -EINVAL;
> -	}
> +
>  	mutex_lock(&indio_dev->mlock);
>  	gpio_set_value(st->pdata->gpio_range, lval == 10000);
>  	st->range = lval;
> @@ -174,11 +173,8 @@ static int ad7606_write_raw(struct iio_dev *indio_dev,
>  		if (val2)
>  			return -EINVAL;
>  		ret = ad7606_oversampling_get_index(val);
> -		if (ret < 0) {
> -			dev_err(st->dev, "oversampling %d is not supported\n",
> -				val);
> +		if (ret < 0)
>  			return ret;
> -		}
>  
>  		mutex_lock(&indio_dev->mlock);
>  		gpio_set_value(st->pdata->gpio_os0, (ret >> 0) & 1);
> 

Re: [PATCH 05/13] staging:iio:ad7606: Use oversampling ratio of 1 for no oversampling

[Jonathan Cameron]

On 19/10/16 18:07, Lars-Peter Clausen wrote:
> Currently the ad7606 driver uses a value of 0 for the oversampling ratio to
> express that no oversampling is done. Strictly speaking this means though
> that no data capture is done at all. Instead change the driver to use a
> value of 1, this is in accordance with what other drivers do and what the
> IIO spec suggests.
> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Applied.

Thanks,

J
> ---
>  drivers/staging/iio/adc/ad7606_core.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/staging/iio/adc/ad7606_core.c b/drivers/staging/iio/adc/ad7606_core.c
> index d3a3551..7c093e2 100644
> --- a/drivers/staging/iio/adc/ad7606_core.c
> +++ b/drivers/staging/iio/adc/ad7606_core.c
> @@ -149,7 +149,7 @@ static IIO_CONST_ATTR(in_voltage_range_available, "5000 10000");
>  
>  static int ad7606_oversampling_get_index(unsigned int val)
>  {
> -	unsigned char supported[] = {0, 2, 4, 8, 16, 32, 64};
> +	unsigned char supported[] = {1, 2, 4, 8, 16, 32, 64};
>  	int i;
>  
>  	for (i = 0; i < ARRAY_SIZE(supported); i++)
> @@ -188,7 +188,7 @@ static int ad7606_write_raw(struct iio_dev *indio_dev,
>  	}
>  }
>  
> -static IIO_CONST_ATTR(oversampling_ratio_available, "0 2 4 8 16 32 64");
> +static IIO_CONST_ATTR(oversampling_ratio_available, "1 2 4 8 16 32 64");
>  
>  static struct attribute *ad7606_attributes_os_and_range[] = {
>  	&iio_dev_attr_in_voltage_range.dev_attr.attr,
> @@ -449,7 +449,7 @@ struct iio_dev *ad7606_probe(struct device *dev, int irq,
>  	st->bops = bops;
>  	st->base_address = base_address;
>  	st->range = 5000;
> -	st->oversampling = 0;
> +	st->oversampling = 1;
>  
>  	st->reg = devm_regulator_get(dev, "vcc");
>  	if (!IS_ERR(st->reg)) {
> 

Re: [PATCH 06/13] staging:iio:ad7606: Avoid allocating buffer for each data capture

[Jonathan Cameron]

On 19/10/16 18:07, Lars-Peter Clausen wrote:
> Currently the ad7606 driver dynamically allocates and frees a transfer
> buffer each time a sample capture is performed in buffered mode, which
> introduces unnecessary overhead. The driver state struct already contains a
> buffer that is used for transfers in one-shot mode. This buffer is large
> enough to hold all samples, but not the timestamp that might be present in
> buffered mode. Extend the buffer size to be able to contain the timestamp
> and update the buffered capture function to use this buffer.
> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Applied. Thanks,

Jonathan
> ---
>  drivers/staging/iio/adc/ad7606.h      |  4 ++--
>  drivers/staging/iio/adc/ad7606_ring.c | 14 ++++----------
>  2 files changed, 6 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/staging/iio/adc/ad7606.h b/drivers/staging/iio/adc/ad7606.h
> index cf6be65..129f94e 100644
> --- a/drivers/staging/iio/adc/ad7606.h
> +++ b/drivers/staging/iio/adc/ad7606.h
> @@ -68,9 +68,9 @@ struct ad7606_state {
>  	/*
>  	 * DMA (thus cache coherency maintenance) requires the
>  	 * transfer buffers to live in their own cache lines.
> +	 * 8 * 16-bit samples + 64-bit timestamp
>  	 */
> -
> -	unsigned short			data[8] ____cacheline_aligned;
> +	unsigned short			data[12] ____cacheline_aligned;
>  };
>  
>  struct ad7606_bus_ops {
> diff --git a/drivers/staging/iio/adc/ad7606_ring.c b/drivers/staging/iio/adc/ad7606_ring.c
> index 0572df9..7fa4ccc 100644
> --- a/drivers/staging/iio/adc/ad7606_ring.c
> +++ b/drivers/staging/iio/adc/ad7606_ring.c
> @@ -46,15 +46,10 @@ static void ad7606_poll_bh_to_ring(struct work_struct *work_s)
>  	struct ad7606_state *st = container_of(work_s, struct ad7606_state,
>  						poll_work);
>  	struct iio_dev *indio_dev = iio_priv_to_dev(st);
> -	__u8 *buf;
>  	int ret;
>  
> -	buf = kzalloc(indio_dev->scan_bytes, GFP_KERNEL);
> -	if (!buf)
> -		return;
> -
>  	if (gpio_is_valid(st->pdata->gpio_frstdata)) {
> -		ret = st->bops->read_block(st->dev, 1, buf);
> +		ret = st->bops->read_block(st->dev, 1, st->data);
>  		if (ret)
>  			goto done;
>  		if (!gpio_get_value(st->pdata->gpio_frstdata)) {
> @@ -67,22 +62,21 @@ static void ad7606_poll_bh_to_ring(struct work_struct *work_s)
>  			goto done;
>  		}
>  		ret = st->bops->read_block(st->dev,
> -			st->chip_info->num_channels - 1, buf + 2);
> +			st->chip_info->num_channels - 1, st->data + 1);
>  		if (ret)
>  			goto done;
>  	} else {
>  		ret = st->bops->read_block(st->dev,
> -			st->chip_info->num_channels, buf);
> +			st->chip_info->num_channels, st->data);
>  		if (ret)
>  			goto done;
>  	}
>  
> -	iio_push_to_buffers_with_timestamp(indio_dev, buf,
> +	iio_push_to_buffers_with_timestamp(indio_dev, st->data,
>  					   iio_get_time_ns(indio_dev));
>  done:
>  	gpio_set_value(st->pdata->gpio_convst, 0);
>  	iio_trigger_notify_done(indio_dev->trig);
> -	kfree(buf);
>  }
>  
>  int ad7606_register_ring_funcs_and_init(struct iio_dev *indio_dev)
> 

Re: [PATCH 08/13] staging:iio:ad7606: Move set_drvdata() into common code

[Jonathan Cameron]

On 19/10/16 18:07, Lars-Peter Clausen wrote:
> Both the platform_device and SPI driver call set_drvdata() at the end of
> their probe function. Move this into the common probe() function to reduce
> duplicated code.
> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Applied.  Thanks

Jonathan
> ---
>  drivers/staging/iio/adc/ad7606_core.c | 2 ++
>  drivers/staging/iio/adc/ad7606_par.c  | 2 --
>  drivers/staging/iio/adc/ad7606_spi.c  | 2 --
>  3 files changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/staging/iio/adc/ad7606_core.c b/drivers/staging/iio/adc/ad7606_core.c
> index 4ce103a..13d8090 100644
> --- a/drivers/staging/iio/adc/ad7606_core.c
> +++ b/drivers/staging/iio/adc/ad7606_core.c
> @@ -517,6 +517,8 @@ struct iio_dev *ad7606_probe(struct device *dev, int irq,
>  	if (ret)
>  		goto error_unregister_ring;
>  
> +	dev_set_drvdata(dev, indio_dev);
> +
>  	return indio_dev;
>  error_unregister_ring:
>  	ad7606_ring_cleanup(indio_dev);
> diff --git a/drivers/staging/iio/adc/ad7606_par.c b/drivers/staging/iio/adc/ad7606_par.c
> index 0a23cd9..c273993 100644
> --- a/drivers/staging/iio/adc/ad7606_par.c
> +++ b/drivers/staging/iio/adc/ad7606_par.c
> @@ -77,8 +77,6 @@ static int ad7606_par_probe(struct platform_device *pdev)
>  	if (IS_ERR(indio_dev))
>  		return PTR_ERR(indio_dev);
>  
> -	platform_set_drvdata(pdev, indio_dev);
> -
>  	return 0;
>  }
>  
> diff --git a/drivers/staging/iio/adc/ad7606_spi.c b/drivers/staging/iio/adc/ad7606_spi.c
> index f69db59..9abbc15 100644
> --- a/drivers/staging/iio/adc/ad7606_spi.c
> +++ b/drivers/staging/iio/adc/ad7606_spi.c
> @@ -52,8 +52,6 @@ static int ad7606_spi_probe(struct spi_device *spi)
>  	if (IS_ERR(indio_dev))
>  		return PTR_ERR(indio_dev);
>  
> -	spi_set_drvdata(spi, indio_dev);
> -
>  	return 0;
>  }
>  
> 

Re: [PATCH 07/13] staging:iio:ad7606: Factor out common code between periodic and one-shot capture

[Jonathan Cameron]

On 19/10/16 18:07, Lars-Peter Clausen wrote:
> Both the periodic buffer based and one-shot sysfs based capture methods
> share a large portion of their code. Factor this out into a common helper
> function.
> 
> Also provide a comment that better explains in more detail what is going on
> in the capture function.
> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Applied.
> ---
>  drivers/staging/iio/adc/ad7606.h      |  1 +
>  drivers/staging/iio/adc/ad7606_core.c | 58 ++++++++++++++++++++++-------------
>  drivers/staging/iio/adc/ad7606_ring.c | 30 +++---------------
>  3 files changed, 41 insertions(+), 48 deletions(-)
> 
> diff --git a/drivers/staging/iio/adc/ad7606.h b/drivers/staging/iio/adc/ad7606.h
> index 129f94e..2257bdb 100644
> --- a/drivers/staging/iio/adc/ad7606.h
> +++ b/drivers/staging/iio/adc/ad7606.h
> @@ -84,6 +84,7 @@ struct iio_dev *ad7606_probe(struct device *dev, int irq,
>  			      const struct ad7606_bus_ops *bops);
>  int ad7606_remove(struct iio_dev *indio_dev, int irq);
>  int ad7606_reset(struct ad7606_state *st);
> +int ad7606_read_samples(struct ad7606_state *st);
>  
>  enum ad7606_supported_device_ids {
>  	ID_AD7606_8,
> diff --git a/drivers/staging/iio/adc/ad7606_core.c b/drivers/staging/iio/adc/ad7606_core.c
> index 7c093e2..4ce103a 100644
> --- a/drivers/staging/iio/adc/ad7606_core.c
> +++ b/drivers/staging/iio/adc/ad7606_core.c
> @@ -36,6 +36,39 @@ int ad7606_reset(struct ad7606_state *st)
>  	return -ENODEV;
>  }
>  
> +int ad7606_read_samples(struct ad7606_state *st)
> +{
> +	unsigned int num = st->chip_info->num_channels;
> +	u16 *data = st->data;
> +	int ret;
> +
> +	/*
> +	 * The frstdata signal is set to high while and after reading the sample
> +	 * of the first channel and low for all other channels. This can be used
> +	 * to check that the incoming data is correctly aligned. During normal
> +	 * operation the data should never become unaligned, but some glitch or
> +	 * electrostatic discharge might cause an extra read or clock cycle.
> +	 * Monitoring the frstdata signal allows to recover from such failure
> +	 * situations.
> +	 */
> +
> +	if (gpio_is_valid(st->pdata->gpio_frstdata)) {
> +		ret = st->bops->read_block(st->dev, 1, data);
> +		if (ret)
> +			return ret;
> +
> +		if (!gpio_get_value(st->pdata->gpio_frstdata)) {
> +			ad7606_reset(st);
> +			return -EIO;
> +		}
> +
> +		data++;
> +		num--;
> +	}
> +
> +	return st->bops->read_block(st->dev, num, data);
> +}
> +
>  static int ad7606_scan_direct(struct iio_dev *indio_dev, unsigned int ch)
>  {
>  	struct ad7606_state *st = iio_priv(indio_dev);
> @@ -48,28 +81,9 @@ static int ad7606_scan_direct(struct iio_dev *indio_dev, unsigned int ch)
>  	if (ret)
>  		goto error_ret;
>  
> -	if (gpio_is_valid(st->pdata->gpio_frstdata)) {
> -		ret = st->bops->read_block(st->dev, 1, st->data);
> -		if (ret)
> -			goto error_ret;
> -		if (!gpio_get_value(st->pdata->gpio_frstdata)) {
> -			/* This should never happen */
> -			ad7606_reset(st);
> -			ret = -EIO;
> -			goto error_ret;
> -		}
> -		ret = st->bops->read_block(st->dev,
> -			st->chip_info->num_channels - 1, &st->data[1]);
> -		if (ret)
> -			goto error_ret;
> -	} else {
> -		ret = st->bops->read_block(st->dev,
> -			st->chip_info->num_channels, st->data);
> -		if (ret)
> -			goto error_ret;
> -	}
> -
> -	ret = st->data[ch];
> +	ret = ad7606_read_samples(st);
> +	if (ret == 0)
> +		ret = st->data[ch];
>  
>  error_ret:
>  	gpio_set_value(st->pdata->gpio_convst, 0);
> diff --git a/drivers/staging/iio/adc/ad7606_ring.c b/drivers/staging/iio/adc/ad7606_ring.c
> index 7fa4ccc..b7bf0cf 100644
> --- a/drivers/staging/iio/adc/ad7606_ring.c
> +++ b/drivers/staging/iio/adc/ad7606_ring.c
> @@ -48,33 +48,11 @@ static void ad7606_poll_bh_to_ring(struct work_struct *work_s)
>  	struct iio_dev *indio_dev = iio_priv_to_dev(st);
>  	int ret;
>  
> -	if (gpio_is_valid(st->pdata->gpio_frstdata)) {
> -		ret = st->bops->read_block(st->dev, 1, st->data);
> -		if (ret)
> -			goto done;
> -		if (!gpio_get_value(st->pdata->gpio_frstdata)) {
> -			/* This should never happen. However
> -			 * some signal glitch caused by bad PCB desgin or
> -			 * electrostatic discharge, could cause an extra read
> -			 * or clock. This allows recovery.
> -			 */
> -			ad7606_reset(st);
> -			goto done;
> -		}
> -		ret = st->bops->read_block(st->dev,
> -			st->chip_info->num_channels - 1, st->data + 1);
> -		if (ret)
> -			goto done;
> -	} else {
> -		ret = st->bops->read_block(st->dev,
> -			st->chip_info->num_channels, st->data);
> -		if (ret)
> -			goto done;
> -	}
> +	ret = ad7606_read_samples(st);
> +	if (ret == 0)
> +		iio_push_to_buffers_with_timestamp(indio_dev, st->data,
> +						   iio_get_time_ns(indio_dev));
>  
> -	iio_push_to_buffers_with_timestamp(indio_dev, st->data,
> -					   iio_get_time_ns(indio_dev));
> -done:
>  	gpio_set_value(st->pdata->gpio_convst, 0);
>  	iio_trigger_notify_done(indio_dev->trig);
>  }
> 

Re: [PATCH 07/13] staging:iio:ad7606: Factor out common code between periodic and one-shot capture

[Jonathan Cameron]

On 19/10/16 18:07, Lars-Peter Clausen wrote:
> Both the periodic buffer based and one-shot sysfs based capture methods
> share a large portion of their code. Factor this out into a common helper
> function.
> 
> Also provide a comment that better explains in more detail what is going on
> in the capture function.
> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Few comments inline, but nothing that actually matters.

Jonathan
> ---
>  drivers/staging/iio/adc/ad7606.h      |  1 +
>  drivers/staging/iio/adc/ad7606_core.c | 58 ++++++++++++++++++++++-------------
>  drivers/staging/iio/adc/ad7606_ring.c | 30 +++---------------
>  3 files changed, 41 insertions(+), 48 deletions(-)
> 
> diff --git a/drivers/staging/iio/adc/ad7606.h b/drivers/staging/iio/adc/ad7606.h
> index 129f94e..2257bdb 100644
> --- a/drivers/staging/iio/adc/ad7606.h
> +++ b/drivers/staging/iio/adc/ad7606.h
> @@ -84,6 +84,7 @@ struct iio_dev *ad7606_probe(struct device *dev, int irq,
>  			      const struct ad7606_bus_ops *bops);
>  int ad7606_remove(struct iio_dev *indio_dev, int irq);
>  int ad7606_reset(struct ad7606_state *st);
> +int ad7606_read_samples(struct ad7606_state *st);
>  
>  enum ad7606_supported_device_ids {
>  	ID_AD7606_8,
> diff --git a/drivers/staging/iio/adc/ad7606_core.c b/drivers/staging/iio/adc/ad7606_core.c
> index 7c093e2..4ce103a 100644
> --- a/drivers/staging/iio/adc/ad7606_core.c
> +++ b/drivers/staging/iio/adc/ad7606_core.c
> @@ -36,6 +36,39 @@ int ad7606_reset(struct ad7606_state *st)
>  	return -ENODEV;
>  }
>  
> +int ad7606_read_samples(struct ad7606_state *st)
> +{
> +	unsigned int num = st->chip_info->num_channels;
> +	u16 *data = st->data;
> +	int ret;
> +
> +	/*
> +	 * The frstdata signal is set to high while and after reading the sample
> +	 * of the first channel and low for all other channels. This can be used
> +	 * to check that the incoming data is correctly aligned. During normal
> +	 * operation the data should never become unaligned, but some glitch or
> +	 * electrostatic discharge might cause an extra read or clock cycle.
> +	 * Monitoring the frstdata signal allows to recover from such failure
> +	 * situations.
> +	 */
Serious paranoia ;)
> +
> +	if (gpio_is_valid(st->pdata->gpio_frstdata)) {
> +		ret = st->bops->read_block(st->dev, 1, data);
> +		if (ret)
> +			return ret;
> +
> +		if (!gpio_get_value(st->pdata->gpio_frstdata)) {
> +			ad7606_reset(st);
> +			return -EIO;
> +		}
> +
> +		data++;
> +		num--;
> +	}
> +
> +	return st->bops->read_block(st->dev, num, data);
> +}
> +
>  static int ad7606_scan_direct(struct iio_dev *indio_dev, unsigned int ch)
>  {
>  	struct ad7606_state *st = iio_priv(indio_dev);
> @@ -48,28 +81,9 @@ static int ad7606_scan_direct(struct iio_dev *indio_dev, unsigned int ch)
>  	if (ret)
>  		goto error_ret;
>  
> -	if (gpio_is_valid(st->pdata->gpio_frstdata)) {
> -		ret = st->bops->read_block(st->dev, 1, st->data);
> -		if (ret)
> -			goto error_ret;
> -		if (!gpio_get_value(st->pdata->gpio_frstdata)) {
> -			/* This should never happen */
> -			ad7606_reset(st);
> -			ret = -EIO;
> -			goto error_ret;
> -		}
> -		ret = st->bops->read_block(st->dev,
> -			st->chip_info->num_channels - 1, &st->data[1]);
> -		if (ret)
> -			goto error_ret;
> -	} else {
> -		ret = st->bops->read_block(st->dev,
> -			st->chip_info->num_channels, st->data);
> -		if (ret)
> -			goto error_ret;
> -	}
> -
> -	ret = st->data[ch];
> +	ret = ad7606_read_samples(st);
> +	if (ret == 0)
> +		ret = st->data[ch];
>  
>  error_ret:
>  	gpio_set_value(st->pdata->gpio_convst, 0);
> diff --git a/drivers/staging/iio/adc/ad7606_ring.c b/drivers/staging/iio/adc/ad7606_ring.c
> index 7fa4ccc..b7bf0cf 100644
> --- a/drivers/staging/iio/adc/ad7606_ring.c
> +++ b/drivers/staging/iio/adc/ad7606_ring.c
> @@ -48,33 +48,11 @@ static void ad7606_poll_bh_to_ring(struct work_struct *work_s)
>  	struct iio_dev *indio_dev = iio_priv_to_dev(st);
>  	int ret;
>  
> -	if (gpio_is_valid(st->pdata->gpio_frstdata)) {
> -		ret = st->bops->read_block(st->dev, 1, st->data);
> -		if (ret)
> -			goto done;
> -		if (!gpio_get_value(st->pdata->gpio_frstdata)) {
> -			/* This should never happen. However
> -			 * some signal glitch caused by bad PCB desgin or
> -			 * electrostatic discharge, could cause an extra read
> -			 * or clock. This allows recovery.
> -			 */
> -			ad7606_reset(st);
> -			goto done;
> -		}
> -		ret = st->bops->read_block(st->dev,
> -			st->chip_info->num_channels - 1, st->data + 1);
> -		if (ret)
> -			goto done;
> -	} else {
> -		ret = st->bops->read_block(st->dev,
> -			st->chip_info->num_channels, st->data);
> -		if (ret)
> -			goto done;
> -	}
> +	ret = ad7606_read_samples(st);
I personally always slightly prefer the 'good' path to not be out of the
'direct' flow.

Still minor point and I don't care that much!
> +	if (ret == 0)
> +		iio_push_to_buffers_with_timestamp(indio_dev, st->data,
> +						   iio_get_time_ns(indio_dev));
>  
> -	iio_push_to_buffers_with_timestamp(indio_dev, st->data,
> -					   iio_get_time_ns(indio_dev));
> -done:
>  	gpio_set_value(st->pdata->gpio_convst, 0);
>  	iio_trigger_notify_done(indio_dev->trig);
>  }
> 

Re: [PATCH 09/13] staging:iio:ad7606: Let the common probe function return int

[Jonathan Cameron]

On 19/10/16 18:07, Lars-Peter Clausen wrote:
> The common probe function for the ad7606 currently returns a struct iio_dev
> pointer. The returned value is not used by the individual driver probe
> functions other than for error checking.
> 
> Let the common probe function return a int instead to report the error
> value directly (or 0 on success). This allows to simplify the code.
> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Applied.  Amazing how this convoluted stuff ends up in drivers
sometimes ;)

Jonathan
> ---
>  drivers/staging/iio/adc/ad7606.h      |  7 +++----
>  drivers/staging/iio/adc/ad7606_core.c | 15 +++++++--------
>  drivers/staging/iio/adc/ad7606_par.c  | 14 ++++----------
>  drivers/staging/iio/adc/ad7606_spi.c  | 12 +++---------
>  4 files changed, 17 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/staging/iio/adc/ad7606.h b/drivers/staging/iio/adc/ad7606.h
> index 2257bdb..b4b25a2 100644
> --- a/drivers/staging/iio/adc/ad7606.h
> +++ b/drivers/staging/iio/adc/ad7606.h
> @@ -78,10 +78,9 @@ struct ad7606_bus_ops {
>  	int (*read_block)(struct device *, int, void *);
>  };
>  
> -struct iio_dev *ad7606_probe(struct device *dev, int irq,
> -			      void __iomem *base_address,
> -			      const char *name, unsigned int id,
> -			      const struct ad7606_bus_ops *bops);
> +int ad7606_probe(struct device *dev, int irq, void __iomem *base_address,
> +		 const char *name, unsigned int id,
> +		 const struct ad7606_bus_ops *bops);
>  int ad7606_remove(struct iio_dev *indio_dev, int irq);
>  int ad7606_reset(struct ad7606_state *st);
>  int ad7606_read_samples(struct ad7606_state *st);
> diff --git a/drivers/staging/iio/adc/ad7606_core.c b/drivers/staging/iio/adc/ad7606_core.c
> index 13d8090..1ee7a29 100644
> --- a/drivers/staging/iio/adc/ad7606_core.c
> +++ b/drivers/staging/iio/adc/ad7606_core.c
> @@ -443,10 +443,9 @@ static const struct iio_info ad7606_info_range = {
>  	.attrs = &ad7606_attribute_group_range,
>  };
>  
> -struct iio_dev *ad7606_probe(struct device *dev, int irq,
> -			     void __iomem *base_address,
> -			     const char *name, unsigned int id,
> -			     const struct ad7606_bus_ops *bops)
> +int ad7606_probe(struct device *dev, int irq, void __iomem *base_address,
> +		 const char *name, unsigned int id,
> +		 const struct ad7606_bus_ops *bops)
>  {
>  	struct ad7606_platform_data *pdata = dev->platform_data;
>  	struct ad7606_state *st;
> @@ -455,7 +454,7 @@ struct iio_dev *ad7606_probe(struct device *dev, int irq,
>  
>  	indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
>  	if (!indio_dev)
> -		return ERR_PTR(-ENOMEM);
> +		return -ENOMEM;
>  
>  	st = iio_priv(indio_dev);
>  
> @@ -469,7 +468,7 @@ struct iio_dev *ad7606_probe(struct device *dev, int irq,
>  	if (!IS_ERR(st->reg)) {
>  		ret = regulator_enable(st->reg);
>  		if (ret)
> -			return ERR_PTR(ret);
> +			return ret;
>  	}
>  
>  	st->pdata = pdata;
> @@ -519,7 +518,7 @@ struct iio_dev *ad7606_probe(struct device *dev, int irq,
>  
>  	dev_set_drvdata(dev, indio_dev);
>  
> -	return indio_dev;
> +	return 0;
>  error_unregister_ring:
>  	ad7606_ring_cleanup(indio_dev);
>  
> @@ -532,7 +531,7 @@ error_free_gpios:
>  error_disable_reg:
>  	if (!IS_ERR(st->reg))
>  		regulator_disable(st->reg);
> -	return ERR_PTR(ret);
> +	return ret;
>  }
>  EXPORT_SYMBOL_GPL(ad7606_probe);
>  
> diff --git a/drivers/staging/iio/adc/ad7606_par.c b/drivers/staging/iio/adc/ad7606_par.c
> index c273993..42eb9e0 100644
> --- a/drivers/staging/iio/adc/ad7606_par.c
> +++ b/drivers/staging/iio/adc/ad7606_par.c
> @@ -51,7 +51,6 @@ static int ad7606_par_probe(struct platform_device *pdev)
>  {
>  	const struct platform_device_id *id = platform_get_device_id(pdev);
>  	struct resource *res;
> -	struct iio_dev *indio_dev;
>  	void __iomem *addr;
>  	resource_size_t remap_size;
>  	int irq;
> @@ -69,15 +68,10 @@ static int ad7606_par_probe(struct platform_device *pdev)
>  
>  	remap_size = resource_size(res);
>  
> -	indio_dev = ad7606_probe(&pdev->dev, irq, addr,
> -				 id->name, id->driver_data,
> -				 remap_size > 1 ? &ad7606_par16_bops :
> -				 &ad7606_par8_bops);
> -
> -	if (IS_ERR(indio_dev))
> -		return PTR_ERR(indio_dev);
> -
> -	return 0;
> +	return ad7606_probe(&pdev->dev, irq, addr,
> +			    id->name, id->driver_data,
> +			    remap_size > 1 ? &ad7606_par16_bops :
> +			    &ad7606_par8_bops);
>  }
>  
>  static int ad7606_par_remove(struct platform_device *pdev)
> diff --git a/drivers/staging/iio/adc/ad7606_spi.c b/drivers/staging/iio/adc/ad7606_spi.c
> index 9abbc15..064c4c2 100644
> --- a/drivers/staging/iio/adc/ad7606_spi.c
> +++ b/drivers/staging/iio/adc/ad7606_spi.c
> @@ -43,16 +43,10 @@ static const struct ad7606_bus_ops ad7606_spi_bops = {
>  static int ad7606_spi_probe(struct spi_device *spi)
>  {
>  	const struct spi_device_id *id = spi_get_device_id(spi);
> -	struct iio_dev *indio_dev;
>  
> -	indio_dev = ad7606_probe(&spi->dev, spi->irq, NULL,
> -				 id->name, id->driver_data,
> -				 &ad7606_spi_bops);
> -
> -	if (IS_ERR(indio_dev))
> -		return PTR_ERR(indio_dev);
> -
> -	return 0;
> +	return ad7606_probe(&spi->dev, spi->irq, NULL,
> +			    id->name, id->driver_data,
> +			    &ad7606_spi_bops);
>  }
>  
>  static int ad7606_spi_remove(struct spi_device *spi)
> 

Re: [PATCH 11/13] staging:iio:ad7606: Run trigger handler only once per trigger event

[Jonathan Cameron]

On 19/10/16 18:07, Lars-Peter Clausen wrote:
> Currently the ad7606 driver installs the same function for the hard-irq and
> threaded trigger handlers. This was introduced in commit 1caf7cb46135
> ("staging:iio:adc:ad7606 Convert to new channel registration method Update
> Add missing call to iio_trigger_notify_done() Set pollfunc top and bottom
> half handler"). Unfortunately the commit message does not mention why this
> was done and Michael does not remember either.
> 
> Since the trigger handler function is idempotent (set a GPIO to 1) running
> it twice does not do any harm, but is simply not necessary either. So set
> the threaded trigger handler for the driver to NULL.
> 
> While we are at it also remove the function description comment that does
> no say anything that can't be derived from the function name itself.
> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Applied.  Thanks.

Jonathan
> ---
>  drivers/staging/iio/adc/ad7606_ring.c | 11 +++--------
>  1 file changed, 3 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/staging/iio/adc/ad7606_ring.c b/drivers/staging/iio/adc/ad7606_ring.c
> index b7bf0cf..81e4a0ac 100644
> --- a/drivers/staging/iio/adc/ad7606_ring.c
> +++ b/drivers/staging/iio/adc/ad7606_ring.c
> @@ -18,11 +18,7 @@
>  
>  #include "ad7606.h"
>  
> -/**
> - * ad7606_trigger_handler_th() th/bh of trigger launched polling to ring buffer
> - *
> - **/
> -static irqreturn_t ad7606_trigger_handler_th_bh(int irq, void *p)
> +static irqreturn_t ad7606_trigger_handler(int irq, void *p)
>  {
>  	struct iio_poll_func *pf = p;
>  	struct ad7606_state *st = iio_priv(pf->indio_dev);
> @@ -63,9 +59,8 @@ int ad7606_register_ring_funcs_and_init(struct iio_dev *indio_dev)
>  
>  	INIT_WORK(&st->poll_work, &ad7606_poll_bh_to_ring);
>  
> -	return iio_triggered_buffer_setup(indio_dev,
> -		&ad7606_trigger_handler_th_bh, &ad7606_trigger_handler_th_bh,
> -		NULL);
> +	return iio_triggered_buffer_setup(indio_dev, &ad7606_trigger_handler,
> +					  NULL, NULL);
>  }
>  
>  void ad7606_ring_cleanup(struct iio_dev *indio_dev)
> 

Re: [PATCH 10/13] staging:iio:ad7606: Let common remove function take a struct device *

[Jonathan Cameron]

On 19/10/16 18:07, Lars-Peter Clausen wrote:
> Currently the common remove function takes a struct iio_dev *. This
> parameter is retrieved by the individual driver remove functions by calling
> get_drvdata() on their device. To simplify the code let the common remove
> function directly take a struct dev * and do the IIO device in retrieval
> the common remove function.
> 
> This also aligns the interface with the common probe function.
> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Applied.

Thanks,

Jonathan
> ---
>  drivers/staging/iio/adc/ad7606.h      | 2 +-
>  drivers/staging/iio/adc/ad7606_core.c | 3 ++-
>  drivers/staging/iio/adc/ad7606_par.c  | 6 +-----
>  drivers/staging/iio/adc/ad7606_spi.c  | 4 +---
>  4 files changed, 5 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/staging/iio/adc/ad7606.h b/drivers/staging/iio/adc/ad7606.h
> index b4b25a2..6dde987 100644
> --- a/drivers/staging/iio/adc/ad7606.h
> +++ b/drivers/staging/iio/adc/ad7606.h
> @@ -81,7 +81,7 @@ struct ad7606_bus_ops {
>  int ad7606_probe(struct device *dev, int irq, void __iomem *base_address,
>  		 const char *name, unsigned int id,
>  		 const struct ad7606_bus_ops *bops);
> -int ad7606_remove(struct iio_dev *indio_dev, int irq);
> +int ad7606_remove(struct device *dev, int irq);
>  int ad7606_reset(struct ad7606_state *st);
>  int ad7606_read_samples(struct ad7606_state *st);
>  
> diff --git a/drivers/staging/iio/adc/ad7606_core.c b/drivers/staging/iio/adc/ad7606_core.c
> index 1ee7a29..670caa2 100644
> --- a/drivers/staging/iio/adc/ad7606_core.c
> +++ b/drivers/staging/iio/adc/ad7606_core.c
> @@ -535,8 +535,9 @@ error_disable_reg:
>  }
>  EXPORT_SYMBOL_GPL(ad7606_probe);
>  
> -int ad7606_remove(struct iio_dev *indio_dev, int irq)
> +int ad7606_remove(struct device *dev, int irq)
>  {
> +	struct iio_dev *indio_dev = dev_get_drvdata(dev);
>  	struct ad7606_state *st = iio_priv(indio_dev);
>  
>  	iio_device_unregister(indio_dev);
> diff --git a/drivers/staging/iio/adc/ad7606_par.c b/drivers/staging/iio/adc/ad7606_par.c
> index 42eb9e0..cd6c410c 100644
> --- a/drivers/staging/iio/adc/ad7606_par.c
> +++ b/drivers/staging/iio/adc/ad7606_par.c
> @@ -76,11 +76,7 @@ static int ad7606_par_probe(struct platform_device *pdev)
>  
>  static int ad7606_par_remove(struct platform_device *pdev)
>  {
> -	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> -
> -	ad7606_remove(indio_dev, platform_get_irq(pdev, 0));
> -
> -	return 0;
> +	return ad7606_remove(&pdev->dev, platform_get_irq(pdev, 0));
>  }
>  
>  static const struct platform_device_id ad7606_driver_ids[] = {
> diff --git a/drivers/staging/iio/adc/ad7606_spi.c b/drivers/staging/iio/adc/ad7606_spi.c
> index 064c4c2..c9b1f266 100644
> --- a/drivers/staging/iio/adc/ad7606_spi.c
> +++ b/drivers/staging/iio/adc/ad7606_spi.c
> @@ -51,9 +51,7 @@ static int ad7606_spi_probe(struct spi_device *spi)
>  
>  static int ad7606_spi_remove(struct spi_device *spi)
>  {
> -	struct iio_dev *indio_dev = dev_get_drvdata(&spi->dev);
> -
> -	return ad7606_remove(indio_dev, spi->irq);
> +	return ad7606_remove(&spi->dev, spi->irq);
>  }
>  
>  static const struct spi_device_id ad7606_id[] = {
> 

Re: [PATCH 12/13] staging:iio:ad7606: Use GPIO descriptor API

[Jonathan Cameron]

On 19/10/16 18:07, Lars-Peter Clausen wrote:
> Convert the ad7606 driver away from the deprecated legacy GPIO API and use
> the new GPIO descriptor API.
> 
> This also means that the platform data struct is now empty and can be
> removed.
> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Nice illustration of how much nicer the newer gpio stuff is.

Applied.

Thanks,

Jonathan
> ---
>  drivers/staging/iio/adc/ad7606.h      |  35 ++----
>  drivers/staging/iio/adc/ad7606_core.c | 200 ++++++++++------------------------
>  drivers/staging/iio/adc/ad7606_ring.c |   4 +-
>  3 files changed, 64 insertions(+), 175 deletions(-)
> 
> diff --git a/drivers/staging/iio/adc/ad7606.h b/drivers/staging/iio/adc/ad7606.h
> index 6dde987..f5f85fa 100644
> --- a/drivers/staging/iio/adc/ad7606.h
> +++ b/drivers/staging/iio/adc/ad7606.h
> @@ -9,33 +9,6 @@
>  #ifndef IIO_ADC_AD7606_H_
>  #define IIO_ADC_AD7606_H_
>  
> -/*
> - * TODO: struct ad7606_platform_data needs to go into include/linux/iio
> - */
> -
> -/**
> - * struct ad7606_platform_data - platform/board specific information
> - * @gpio_convst:	number of gpio connected to the CONVST pin
> - * @gpio_reset:		gpio connected to the RESET pin, if not used set to -1
> - * @gpio_range:		gpio connected to the RANGE pin, if not used set to -1
> - * @gpio_os0:		gpio connected to the OS0 pin, if not used set to -1
> - * @gpio_os1:		gpio connected to the OS1 pin, if not used set to -1
> - * @gpio_os2:		gpio connected to the OS2 pin, if not used set to -1
> - * @gpio_frstdata:	gpio connected to the FRSTDAT pin, if not used set to -1
> - * @gpio_stby:		gpio connected to the STBY pin, if not used set to -1
> - */
> -
> -struct ad7606_platform_data {
> -	unsigned int			gpio_convst;
> -	unsigned int			gpio_reset;
> -	unsigned int			gpio_range;
> -	unsigned int			gpio_os0;
> -	unsigned int			gpio_os1;
> -	unsigned int			gpio_os2;
> -	unsigned int			gpio_frstdata;
> -	unsigned int			gpio_stby;
> -};
> -
>  /**
>   * struct ad7606_chip_info - chip specific information
>   * @name:		identification string for chip
> @@ -55,7 +28,6 @@ struct ad7606_chip_info {
>  struct ad7606_state {
>  	struct device			*dev;
>  	const struct ad7606_chip_info	*chip_info;
> -	struct ad7606_platform_data	*pdata;
>  	struct regulator		*reg;
>  	struct work_struct		poll_work;
>  	wait_queue_head_t		wq_data_avail;
> @@ -65,6 +37,13 @@ struct ad7606_state {
>  	bool				done;
>  	void __iomem			*base_address;
>  
> +	struct gpio_desc		*gpio_convst;
> +	struct gpio_desc		*gpio_reset;
> +	struct gpio_desc		*gpio_range;
> +	struct gpio_desc		*gpio_standby;
> +	struct gpio_desc		*gpio_frstdata;
> +	struct gpio_descs		*gpio_os;
> +
>  	/*
>  	 * DMA (thus cache coherency maintenance) requires the
>  	 * transfer buffers to live in their own cache lines.
> diff --git a/drivers/staging/iio/adc/ad7606_core.c b/drivers/staging/iio/adc/ad7606_core.c
> index 670caa2..4ef6cb4 100644
> --- a/drivers/staging/iio/adc/ad7606_core.c
> +++ b/drivers/staging/iio/adc/ad7606_core.c
> @@ -13,7 +13,7 @@
>  #include <linux/sysfs.h>
>  #include <linux/regulator/consumer.h>
>  #include <linux/err.h>
> -#include <linux/gpio.h>
> +#include <linux/gpio/consumer.h>
>  #include <linux/delay.h>
>  #include <linux/sched.h>
>  #include <linux/module.h>
> @@ -26,10 +26,10 @@
>  
>  int ad7606_reset(struct ad7606_state *st)
>  {
> -	if (gpio_is_valid(st->pdata->gpio_reset)) {
> -		gpio_set_value(st->pdata->gpio_reset, 1);
> +	if (st->gpio_reset) {
> +		gpiod_set_value(st->gpio_reset, 1);
>  		ndelay(100); /* t_reset >= 100ns */
> -		gpio_set_value(st->pdata->gpio_reset, 0);
> +		gpiod_set_value(st->gpio_reset, 0);
>  		return 0;
>  	}
>  
> @@ -52,12 +52,12 @@ int ad7606_read_samples(struct ad7606_state *st)
>  	 * situations.
>  	 */
>  
> -	if (gpio_is_valid(st->pdata->gpio_frstdata)) {
> +	if (st->gpio_frstdata) {
>  		ret = st->bops->read_block(st->dev, 1, data);
>  		if (ret)
>  			return ret;
>  
> -		if (!gpio_get_value(st->pdata->gpio_frstdata)) {
> +		if (!gpiod_get_value(st->gpio_frstdata)) {
>  			ad7606_reset(st);
>  			return -EIO;
>  		}
> @@ -75,7 +75,7 @@ static int ad7606_scan_direct(struct iio_dev *indio_dev, unsigned int ch)
>  	int ret;
>  
>  	st->done = false;
> -	gpio_set_value(st->pdata->gpio_convst, 1);
> +	gpiod_set_value(st->gpio_convst, 1);
>  
>  	ret = wait_event_interruptible(st->wq_data_avail, st->done);
>  	if (ret)
> @@ -86,7 +86,7 @@ static int ad7606_scan_direct(struct iio_dev *indio_dev, unsigned int ch)
>  		ret = st->data[ch];
>  
>  error_ret:
> -	gpio_set_value(st->pdata->gpio_convst, 0);
> +	gpiod_set_value(st->gpio_convst, 0);
>  
>  	return ret;
>  }
> @@ -150,7 +150,7 @@ static ssize_t ad7606_store_range(struct device *dev,
>  		return -EINVAL;
>  
>  	mutex_lock(&indio_dev->mlock);
> -	gpio_set_value(st->pdata->gpio_range, lval == 10000);
> +	gpiod_set_value(st->gpio_range, lval == 10000);
>  	st->range = lval;
>  	mutex_unlock(&indio_dev->mlock);
>  
> @@ -180,6 +180,7 @@ static int ad7606_write_raw(struct iio_dev *indio_dev,
>  			    long mask)
>  {
>  	struct ad7606_state *st = iio_priv(indio_dev);
> +	int values[3];
>  	int ret;
>  
>  	switch (mask) {
> @@ -190,12 +191,16 @@ static int ad7606_write_raw(struct iio_dev *indio_dev,
>  		if (ret < 0)
>  			return ret;
>  
> +		values[0] = (ret >> 0) & 1;
> +		values[1] = (ret >> 1) & 1;
> +		values[2] = (ret >> 2) & 1;
> +
>  		mutex_lock(&indio_dev->mlock);
> -		gpio_set_value(st->pdata->gpio_os0, (ret >> 0) & 1);
> -		gpio_set_value(st->pdata->gpio_os1, (ret >> 1) & 1);
> -		gpio_set_value(st->pdata->gpio_os2, (ret >> 2) & 1);
> +		gpiod_set_array_value(ARRAY_SIZE(values), st->gpio_os->desc,
> +				      values);
>  		st->oversampling = val;
>  		mutex_unlock(&indio_dev->mlock);
> +
>  		return 0;
>  	default:
>  		return -EINVAL;
> @@ -285,119 +290,37 @@ static const struct ad7606_chip_info ad7606_chip_info_tbl[] = {
>  
>  static int ad7606_request_gpios(struct ad7606_state *st)
>  {
> -	struct gpio gpio_array[3] = {
> -		[0] = {
> -			.gpio =  st->pdata->gpio_os0,
> -			.flags = GPIOF_DIR_OUT | ((st->oversampling & 1) ?
> -				 GPIOF_INIT_HIGH : GPIOF_INIT_LOW),
> -			.label = "AD7606_OS0",
> -		},
> -		[1] = {
> -			.gpio =  st->pdata->gpio_os1,
> -			.flags = GPIOF_DIR_OUT | ((st->oversampling & 2) ?
> -				 GPIOF_INIT_HIGH : GPIOF_INIT_LOW),
> -			.label = "AD7606_OS1",
> -		},
> -		[2] = {
> -			.gpio =  st->pdata->gpio_os2,
> -			.flags = GPIOF_DIR_OUT | ((st->oversampling & 4) ?
> -				 GPIOF_INIT_HIGH : GPIOF_INIT_LOW),
> -			.label = "AD7606_OS2",
> -		},
> -	};
> -	int ret;
> +	struct device *dev = st->dev;
>  
> -	if (gpio_is_valid(st->pdata->gpio_convst)) {
> -		ret = gpio_request_one(st->pdata->gpio_convst,
> -				       GPIOF_OUT_INIT_LOW,
> -				       "AD7606_CONVST");
> -		if (ret) {
> -			dev_err(st->dev, "failed to request GPIO CONVST\n");
> -			goto error_ret;
> -		}
> -	} else {
> -		ret = -EIO;
> -		goto error_ret;
> -	}
> +	st->gpio_convst = devm_gpiod_get(dev, "conversion-start",
> +					 GPIOD_OUT_LOW);
> +	if (IS_ERR(st->gpio_convst))
> +		return PTR_ERR(st->gpio_convst);
>  
> -	if (gpio_is_valid(st->pdata->gpio_os0) &&
> -	    gpio_is_valid(st->pdata->gpio_os1) &&
> -	    gpio_is_valid(st->pdata->gpio_os2)) {
> -		ret = gpio_request_array(gpio_array, ARRAY_SIZE(gpio_array));
> -		if (ret < 0)
> -			goto error_free_convst;
> -	}
> +	st->gpio_reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
> +	if (IS_ERR(st->gpio_reset))
> +		return PTR_ERR(st->gpio_reset);
>  
> -	if (gpio_is_valid(st->pdata->gpio_reset)) {
> -		ret = gpio_request_one(st->pdata->gpio_reset,
> -				       GPIOF_OUT_INIT_LOW,
> -				       "AD7606_RESET");
> -		if (ret < 0)
> -			goto error_free_os;
> -	}
> +	st->gpio_range = devm_gpiod_get_optional(dev, "range", GPIOD_OUT_LOW);
> +	if (IS_ERR(st->gpio_range))
> +		return PTR_ERR(st->gpio_range);
>  
> -	if (gpio_is_valid(st->pdata->gpio_range)) {
> -		ret = gpio_request_one(st->pdata->gpio_range, GPIOF_DIR_OUT |
> -				       ((st->range == 10000) ? GPIOF_INIT_HIGH :
> -					GPIOF_INIT_LOW), "AD7606_RANGE");
> -		if (ret < 0)
> -			goto error_free_reset;
> -	}
> -	if (gpio_is_valid(st->pdata->gpio_stby)) {
> -		ret = gpio_request_one(st->pdata->gpio_stby,
> -				       GPIOF_OUT_INIT_HIGH,
> -				       "AD7606_STBY");
> -		if (ret < 0)
> -			goto error_free_range;
> -	}
> +	st->gpio_standby = devm_gpiod_get_optional(dev, "standby",
> +						   GPIOD_OUT_HIGH);
> +	if (IS_ERR(st->gpio_standby))
> +		return PTR_ERR(st->gpio_standby);
>  
> -	if (gpio_is_valid(st->pdata->gpio_frstdata)) {
> -		ret = gpio_request_one(st->pdata->gpio_frstdata, GPIOF_IN,
> -				       "AD7606_FRSTDATA");
> -		if (ret < 0)
> -			goto error_free_stby;
> -	}
> +	st->gpio_frstdata = devm_gpiod_get_optional(dev, "first-data",
> +						    GPIOD_IN);
> +	if (IS_ERR(st->gpio_frstdata))
> +		return PTR_ERR(st->gpio_frstdata);
>  
> -	return 0;
> +	st->gpio_os = devm_gpiod_get_array_optional(dev, "oversampling-ratio",
> +			GPIOD_OUT_LOW);
> +	if (IS_ERR(st->gpio_os))
> +		return PTR_ERR(st->gpio_os);
>  
> -error_free_stby:
> -	if (gpio_is_valid(st->pdata->gpio_stby))
> -		gpio_free(st->pdata->gpio_stby);
> -error_free_range:
> -	if (gpio_is_valid(st->pdata->gpio_range))
> -		gpio_free(st->pdata->gpio_range);
> -error_free_reset:
> -	if (gpio_is_valid(st->pdata->gpio_reset))
> -		gpio_free(st->pdata->gpio_reset);
> -error_free_os:
> -	if (gpio_is_valid(st->pdata->gpio_os0) &&
> -	    gpio_is_valid(st->pdata->gpio_os1) &&
> -	    gpio_is_valid(st->pdata->gpio_os2))
> -		gpio_free_array(gpio_array, ARRAY_SIZE(gpio_array));
> -error_free_convst:
> -	gpio_free(st->pdata->gpio_convst);
> -error_ret:
> -	return ret;
> -}
> -
> -static void ad7606_free_gpios(struct ad7606_state *st)
> -{
> -	if (gpio_is_valid(st->pdata->gpio_frstdata))
> -		gpio_free(st->pdata->gpio_frstdata);
> -	if (gpio_is_valid(st->pdata->gpio_stby))
> -		gpio_free(st->pdata->gpio_stby);
> -	if (gpio_is_valid(st->pdata->gpio_range))
> -		gpio_free(st->pdata->gpio_range);
> -	if (gpio_is_valid(st->pdata->gpio_reset))
> -		gpio_free(st->pdata->gpio_reset);
> -	if (gpio_is_valid(st->pdata->gpio_os0) &&
> -	    gpio_is_valid(st->pdata->gpio_os1) &&
> -	    gpio_is_valid(st->pdata->gpio_os2)) {
> -		gpio_free(st->pdata->gpio_os2);
> -		gpio_free(st->pdata->gpio_os1);
> -		gpio_free(st->pdata->gpio_os0);
> -	}
> -	gpio_free(st->pdata->gpio_convst);
> +	return 0;
>  }
>  
>  /**
> @@ -447,7 +370,6 @@ int ad7606_probe(struct device *dev, int irq, void __iomem *base_address,
>  		 const char *name, unsigned int id,
>  		 const struct ad7606_bus_ops *bops)
>  {
> -	struct ad7606_platform_data *pdata = dev->platform_data;
>  	struct ad7606_state *st;
>  	int ret;
>  	struct iio_dev *indio_dev;
> @@ -471,19 +393,20 @@ int ad7606_probe(struct device *dev, int irq, void __iomem *base_address,
>  			return ret;
>  	}
>  
> -	st->pdata = pdata;
> +	ret = ad7606_request_gpios(st);
> +	if (ret)
> +		goto error_disable_reg;
> +
>  	st->chip_info = &ad7606_chip_info_tbl[id];
>  
>  	indio_dev->dev.parent = dev;
> -	if (gpio_is_valid(st->pdata->gpio_os0) &&
> -	    gpio_is_valid(st->pdata->gpio_os1) &&
> -	    gpio_is_valid(st->pdata->gpio_os2)) {
> -		if (gpio_is_valid(st->pdata->gpio_range))
> +	if (st->gpio_os) {
> +		if (st->gpio_range)
>  			indio_dev->info = &ad7606_info_os_and_range;
>  		else
>  			indio_dev->info = &ad7606_info_os;
>  	} else {
> -		if (gpio_is_valid(st->pdata->gpio_range))
> +		if (st->gpio_range)
>  			indio_dev->info = &ad7606_info_range;
>  		else
>  			indio_dev->info = &ad7606_info_no_os_or_range;
> @@ -495,10 +418,6 @@ int ad7606_probe(struct device *dev, int irq, void __iomem *base_address,
>  
>  	init_waitqueue_head(&st->wq_data_avail);
>  
> -	ret = ad7606_request_gpios(st);
> -	if (ret)
> -		goto error_disable_reg;
> -
>  	ret = ad7606_reset(st);
>  	if (ret)
>  		dev_warn(st->dev, "failed to RESET: no RESET GPIO specified\n");
> @@ -506,7 +425,7 @@ int ad7606_probe(struct device *dev, int irq, void __iomem *base_address,
>  	ret = request_irq(irq, ad7606_interrupt, IRQF_TRIGGER_FALLING, name,
>  			  indio_dev);
>  	if (ret)
> -		goto error_free_gpios;
> +		goto error_disable_reg;
>  
>  	ret = ad7606_register_ring_funcs_and_init(indio_dev);
>  	if (ret)
> @@ -525,9 +444,6 @@ error_unregister_ring:
>  error_free_irq:
>  	free_irq(irq, indio_dev);
>  
> -error_free_gpios:
> -	ad7606_free_gpios(st);
> -
>  error_disable_reg:
>  	if (!IS_ERR(st->reg))
>  		regulator_disable(st->reg);
> @@ -547,8 +463,6 @@ int ad7606_remove(struct device *dev, int irq)
>  	if (!IS_ERR(st->reg))
>  		regulator_disable(st->reg);
>  
> -	ad7606_free_gpios(st);
> -
>  	return 0;
>  }
>  EXPORT_SYMBOL_GPL(ad7606_remove);
> @@ -560,10 +474,9 @@ static int ad7606_suspend(struct device *dev)
>  	struct iio_dev *indio_dev = dev_get_drvdata(dev);
>  	struct ad7606_state *st = iio_priv(indio_dev);
>  
> -	if (gpio_is_valid(st->pdata->gpio_stby)) {
> -		if (gpio_is_valid(st->pdata->gpio_range))
> -			gpio_set_value(st->pdata->gpio_range, 1);
> -		gpio_set_value(st->pdata->gpio_stby, 0);
> +	if (st->gpio_standby) {
> +		gpiod_set_value(st->gpio_range, 1);
> +		gpiod_set_value(st->gpio_standby, 0);
>  	}
>  
>  	return 0;
> @@ -574,12 +487,9 @@ static int ad7606_resume(struct device *dev)
>  	struct iio_dev *indio_dev = dev_get_drvdata(dev);
>  	struct ad7606_state *st = iio_priv(indio_dev);
>  
> -	if (gpio_is_valid(st->pdata->gpio_stby)) {
> -		if (gpio_is_valid(st->pdata->gpio_range))
> -			gpio_set_value(st->pdata->gpio_range,
> -				       st->range == 10000);
> -
> -		gpio_set_value(st->pdata->gpio_stby, 1);
> +	if (st->gpio_standby) {
> +		gpiod_set_value(st->gpio_range, st->range == 10000);
> +		gpiod_set_value(st->gpio_standby, 1);
>  		ad7606_reset(st);
>  	}
>  
> diff --git a/drivers/staging/iio/adc/ad7606_ring.c b/drivers/staging/iio/adc/ad7606_ring.c
> index 81e4a0ac..68d72b1 100644
> --- a/drivers/staging/iio/adc/ad7606_ring.c
> +++ b/drivers/staging/iio/adc/ad7606_ring.c
> @@ -23,7 +23,7 @@ static irqreturn_t ad7606_trigger_handler(int irq, void *p)
>  	struct iio_poll_func *pf = p;
>  	struct ad7606_state *st = iio_priv(pf->indio_dev);
>  
> -	gpio_set_value(st->pdata->gpio_convst, 1);
> +	gpiod_set_value(st->gpio_convst, 1);
>  
>  	return IRQ_HANDLED;
>  }
> @@ -49,7 +49,7 @@ static void ad7606_poll_bh_to_ring(struct work_struct *work_s)
>  		iio_push_to_buffers_with_timestamp(indio_dev, st->data,
>  						   iio_get_time_ns(indio_dev));
>  
> -	gpio_set_value(st->pdata->gpio_convst, 0);
> +	gpiod_set_value(st->gpio_convst, 0);
>  	iio_trigger_notify_done(indio_dev->trig);
>  }
>  
> 

Re: [PATCH 13/13] staging:iio:ad7606: Move buffer code to main source file

[Jonathan Cameron]

On 19/10/16 18:07, Lars-Peter Clausen wrote:
> Currently the ad7606 buffer handling code resides in its own source file.
> But this file contains only 4 small functions of which half are just
> wrappers around other functions. Buffer support is also always enabled for
> this driver, so move them over to the main source file. This reduces the
> amount of boilerplate code.
> 
> Also rename the main function from ad7606_core.c to ad7606.c since there is
> only a single file now.
> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Applied.  

A nice bit of cleanup work. It'll stop me moaning about the other
ADI drivers you made the error of taking responsibility for that
are still in staging ;) (for a week or so anyway)

Jonathan
> ---
>  drivers/staging/iio/adc/Makefile                   |  1 -
>  .../staging/iio/adc/{ad7606_core.c => ad7606.c}    | 49 +++++++++++++--
>  drivers/staging/iio/adc/ad7606.h                   |  5 --
>  drivers/staging/iio/adc/ad7606_ring.c              | 69 ----------------------
>  4 files changed, 44 insertions(+), 80 deletions(-)
>  rename drivers/staging/iio/adc/{ad7606_core.c => ad7606.c} (88%)
>  delete mode 100644 drivers/staging/iio/adc/ad7606_ring.c
> 
> diff --git a/drivers/staging/iio/adc/Makefile b/drivers/staging/iio/adc/Makefile
> index 3cdd83c..ac09485 100644
> --- a/drivers/staging/iio/adc/Makefile
> +++ b/drivers/staging/iio/adc/Makefile
> @@ -2,7 +2,6 @@
>  # Makefile for industrial I/O ADC drivers
>  #
>  
> -ad7606-y := ad7606_core.o ad7606_ring.o
>  obj-$(CONFIG_AD7606_IFACE_PARALLEL) += ad7606_par.o
>  obj-$(CONFIG_AD7606_IFACE_SPI) += ad7606_spi.o
>  obj-$(CONFIG_AD7606) += ad7606.o
> diff --git a/drivers/staging/iio/adc/ad7606_core.c b/drivers/staging/iio/adc/ad7606.c
> similarity index 88%
> rename from drivers/staging/iio/adc/ad7606_core.c
> rename to drivers/staging/iio/adc/ad7606.c
> index 4ef6cb4..010c6e1 100644
> --- a/drivers/staging/iio/adc/ad7606_core.c
> +++ b/drivers/staging/iio/adc/ad7606.c
> @@ -21,10 +21,12 @@
>  #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 "ad7606.h"
>  
> -int ad7606_reset(struct ad7606_state *st)
> +static int ad7606_reset(struct ad7606_state *st)
>  {
>  	if (st->gpio_reset) {
>  		gpiod_set_value(st->gpio_reset, 1);
> @@ -36,7 +38,7 @@ int ad7606_reset(struct ad7606_state *st)
>  	return -ENODEV;
>  }
>  
> -int ad7606_read_samples(struct ad7606_state *st)
> +static int ad7606_read_samples(struct ad7606_state *st)
>  {
>  	unsigned int num = st->chip_info->num_channels;
>  	u16 *data = st->data;
> @@ -69,6 +71,41 @@ int ad7606_read_samples(struct ad7606_state *st)
>  	return st->bops->read_block(st->dev, num, data);
>  }
>  
> +static irqreturn_t ad7606_trigger_handler(int irq, void *p)
> +{
> +	struct iio_poll_func *pf = p;
> +	struct ad7606_state *st = iio_priv(pf->indio_dev);
> +
> +	gpiod_set_value(st->gpio_convst, 1);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +/**
> + * ad7606_poll_bh_to_ring() bh of trigger launched polling to ring buffer
> + * @work_s:	the work struct through which this was scheduled
> + *
> + * Currently there is no option in this driver to disable the saving of
> + * timestamps within the ring.
> + * I think the one copy of this at a time was to avoid problems if the
> + * trigger was set far too high and the reads then locked up the computer.
> + **/
> +static void ad7606_poll_bh_to_ring(struct work_struct *work_s)
> +{
> +	struct ad7606_state *st = container_of(work_s, struct ad7606_state,
> +						poll_work);
> +	struct iio_dev *indio_dev = iio_priv_to_dev(st);
> +	int ret;
> +
> +	ret = ad7606_read_samples(st);
> +	if (ret == 0)
> +		iio_push_to_buffers_with_timestamp(indio_dev, st->data,
> +						   iio_get_time_ns(indio_dev));
> +
> +	gpiod_set_value(st->gpio_convst, 0);
> +	iio_trigger_notify_done(indio_dev->trig);
> +}
> +
>  static int ad7606_scan_direct(struct iio_dev *indio_dev, unsigned int ch)
>  {
>  	struct ad7606_state *st = iio_priv(indio_dev);
> @@ -385,6 +422,7 @@ int ad7606_probe(struct device *dev, int irq, void __iomem *base_address,
>  	st->base_address = base_address;
>  	st->range = 5000;
>  	st->oversampling = 1;
> +	INIT_WORK(&st->poll_work, &ad7606_poll_bh_to_ring);
>  
>  	st->reg = devm_regulator_get(dev, "vcc");
>  	if (!IS_ERR(st->reg)) {
> @@ -427,7 +465,8 @@ int ad7606_probe(struct device *dev, int irq, void __iomem *base_address,
>  	if (ret)
>  		goto error_disable_reg;
>  
> -	ret = ad7606_register_ring_funcs_and_init(indio_dev);
> +	ret = iio_triggered_buffer_setup(indio_dev, &ad7606_trigger_handler,
> +					 NULL, NULL);
>  	if (ret)
>  		goto error_free_irq;
>  
> @@ -439,7 +478,7 @@ int ad7606_probe(struct device *dev, int irq, void __iomem *base_address,
>  
>  	return 0;
>  error_unregister_ring:
> -	ad7606_ring_cleanup(indio_dev);
> +	iio_triggered_buffer_cleanup(indio_dev);
>  
>  error_free_irq:
>  	free_irq(irq, indio_dev);
> @@ -457,7 +496,7 @@ int ad7606_remove(struct device *dev, int irq)
>  	struct ad7606_state *st = iio_priv(indio_dev);
>  
>  	iio_device_unregister(indio_dev);
> -	ad7606_ring_cleanup(indio_dev);
> +	iio_triggered_buffer_cleanup(indio_dev);
>  
>  	free_irq(irq, indio_dev);
>  	if (!IS_ERR(st->reg))
> diff --git a/drivers/staging/iio/adc/ad7606.h b/drivers/staging/iio/adc/ad7606.h
> index f5f85fa..746f955 100644
> --- a/drivers/staging/iio/adc/ad7606.h
> +++ b/drivers/staging/iio/adc/ad7606.h
> @@ -61,8 +61,6 @@ int ad7606_probe(struct device *dev, int irq, void __iomem *base_address,
>  		 const char *name, unsigned int id,
>  		 const struct ad7606_bus_ops *bops);
>  int ad7606_remove(struct device *dev, int irq);
> -int ad7606_reset(struct ad7606_state *st);
> -int ad7606_read_samples(struct ad7606_state *st);
>  
>  enum ad7606_supported_device_ids {
>  	ID_AD7606_8,
> @@ -70,9 +68,6 @@ enum ad7606_supported_device_ids {
>  	ID_AD7606_4
>  };
>  
> -int ad7606_register_ring_funcs_and_init(struct iio_dev *indio_dev);
> -void ad7606_ring_cleanup(struct iio_dev *indio_dev);
> -
>  #ifdef CONFIG_PM_SLEEP
>  extern const struct dev_pm_ops ad7606_pm_ops;
>  #define AD7606_PM_OPS (&ad7606_pm_ops)
> diff --git a/drivers/staging/iio/adc/ad7606_ring.c b/drivers/staging/iio/adc/ad7606_ring.c
> deleted file mode 100644
> index 68d72b1..0000000
> --- a/drivers/staging/iio/adc/ad7606_ring.c
> +++ /dev/null
> @@ -1,69 +0,0 @@
> -/*
> - * Copyright 2011-2012 Analog Devices Inc.
> - *
> - * Licensed under the GPL-2.
> - *
> - */
> -
> -#include <linux/interrupt.h>
> -#include <linux/gpio.h>
> -#include <linux/device.h>
> -#include <linux/kernel.h>
> -#include <linux/slab.h>
> -
> -#include <linux/iio/iio.h>
> -#include <linux/iio/buffer.h>
> -#include <linux/iio/trigger_consumer.h>
> -#include <linux/iio/triggered_buffer.h>
> -
> -#include "ad7606.h"
> -
> -static irqreturn_t ad7606_trigger_handler(int irq, void *p)
> -{
> -	struct iio_poll_func *pf = p;
> -	struct ad7606_state *st = iio_priv(pf->indio_dev);
> -
> -	gpiod_set_value(st->gpio_convst, 1);
> -
> -	return IRQ_HANDLED;
> -}
> -
> -/**
> - * ad7606_poll_bh_to_ring() bh of trigger launched polling to ring buffer
> - * @work_s:	the work struct through which this was scheduled
> - *
> - * Currently there is no option in this driver to disable the saving of
> - * timestamps within the ring.
> - * I think the one copy of this at a time was to avoid problems if the
> - * trigger was set far too high and the reads then locked up the computer.
> - **/
> -static void ad7606_poll_bh_to_ring(struct work_struct *work_s)
> -{
> -	struct ad7606_state *st = container_of(work_s, struct ad7606_state,
> -						poll_work);
> -	struct iio_dev *indio_dev = iio_priv_to_dev(st);
> -	int ret;
> -
> -	ret = ad7606_read_samples(st);
> -	if (ret == 0)
> -		iio_push_to_buffers_with_timestamp(indio_dev, st->data,
> -						   iio_get_time_ns(indio_dev));
> -
> -	gpiod_set_value(st->gpio_convst, 0);
> -	iio_trigger_notify_done(indio_dev->trig);
> -}
> -
> -int ad7606_register_ring_funcs_and_init(struct iio_dev *indio_dev)
> -{
> -	struct ad7606_state *st = iio_priv(indio_dev);
> -
> -	INIT_WORK(&st->poll_work, &ad7606_poll_bh_to_ring);
> -
> -	return iio_triggered_buffer_setup(indio_dev, &ad7606_trigger_handler,
> -					  NULL, NULL);
> -}
> -
> -void ad7606_ring_cleanup(struct iio_dev *indio_dev)
> -{
> -	iio_triggered_buffer_cleanup(indio_dev);
> -}
> 

Re: [PATCH 07/13] staging:iio:ad7606: Factor out common code between periodic and one-shot capture

[Lars-Peter Clausen]

On 10/22/2016 07:02 PM, Jonathan Cameron wrote:
[...]
>> +	ret = ad7606_read_samples(st);
> I personally always slightly prefer the 'good' path to not be out of the
> 'direct' flow.
> 
> Still minor point and I don't care that much!

Somewhat agreed, but if I did

	if (ret)
		goto done;
	iio_push_to_buffers_with_timestamp(...)

	done:

It would probably be 5 minutes until somebody sends a cocci generated patch
to 'fix' this ;)

>> +	if (ret == 0)
>> +		iio_push_to_buffers_with_timestamp(indio_dev, st->data,
>> +						   iio_get_time_ns(indio_dev));
>>  
>> -	iio_push_to_buffers_with_timestamp(indio_dev, st->data,
>> -					   iio_get_time_ns(indio_dev));
>> -done:
>>  	gpio_set_value(st->pdata->gpio_convst, 0);
>>  	iio_trigger_notify_done(indio_dev->trig);
>>  }
>>
> 

Re: [PATCH 07/13] staging:iio:ad7606: Factor out common code between periodic and one-shot capture

[Jonathan Cameron]

On 22/10/16 18:20, Lars-Peter Clausen wrote:
> On 10/22/2016 07:02 PM, Jonathan Cameron wrote:
> [...]
>>> +	ret = ad7606_read_samples(st);
>> I personally always slightly prefer the 'good' path to not be out of the
>> 'direct' flow.
>>
>> Still minor point and I don't care that much!
> 
> Somewhat agreed, but if I did
> 
> 	if (ret)
> 		goto done;
> 	iio_push_to_buffers_with_timestamp(...)
> 
> 	done:
> 
> It would probably be 5 minutes until somebody sends a cocci generated patch
> to 'fix' this ;)
I'm good at ignoring those ones ;)

J
> 
>>> +	if (ret == 0)
>>> +		iio_push_to_buffers_with_timestamp(indio_dev, st->data,
>>> +						   iio_get_time_ns(indio_dev));
>>>  
>>> -	iio_push_to_buffers_with_timestamp(indio_dev, st->data,
>>> -					   iio_get_time_ns(indio_dev));
>>> -done:
>>>  	gpio_set_value(st->pdata->gpio_convst, 0);
>>>  	iio_trigger_notify_done(indio_dev->trig);
>>>  }
>>>
>>
> 
> --
> 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
>