[PATCH 1/4] iio: adc: ad7476: Generate CONVST signal internally

[PATCH 1/4] iio: adc: ad7476: Generate CONVST signal internally

[Beniamin Bia]

From: Dragos Bogdan <dragos.bogdan@analog.com>

Compared to the other supported parts, AD7091R are dependent of
a CONVST signal that initiates the conversion. At this moment, only
sampling in buffered mode is supported for AD7091R and the only
option until now was to generate this signal externally using an
IIO trigger. This patch adds the option of generating it internally,
more compatible triggers being available in this case.

Also, it is an intermiate step of adding support more devices.

Signed-off-by: Dragos Bogdan <dragos.bogdan@analog.com>
Signed-off-by: Beniamin Bia <beniamin.bia@analog.com>
---
 drivers/iio/adc/ad7476.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/drivers/iio/adc/ad7476.c b/drivers/iio/adc/ad7476.c
index 76747488044b..32e857dfec9c 100644
--- a/drivers/iio/adc/ad7476.c
+++ b/drivers/iio/adc/ad7476.c
@@ -12,9 +12,11 @@
 #include <linux/sysfs.h>
 #include <linux/spi/spi.h>
 #include <linux/regulator/consumer.h>
+#include <linux/gpio/consumer.h>
 #include <linux/err.h>
 #include <linux/module.h>
 #include <linux/bitops.h>
+#include <linux/delay.h>
 
 #include <linux/iio/iio.h>
 #include <linux/iio/sysfs.h>
@@ -34,6 +36,7 @@ struct ad7476_state {
 	struct spi_device		*spi;
 	const struct ad7476_chip_info	*chip_info;
 	struct regulator		*reg;
+	struct gpio_desc		*convst_gpio;
 	struct spi_transfer		xfer;
 	struct spi_message		msg;
 	/*
@@ -64,6 +67,17 @@ enum ad7476_supported_device_ids {
 	ID_ADS7868,
 };
 
+static void ad7091_convst(struct ad7476_state *st)
+{
+	if (!st->convst_gpio)
+		return;
+
+	gpiod_set_value(st->convst_gpio, 0);
+	udelay(1); /* CONVST pulse width: 10 ns min */
+	gpiod_set_value(st->convst_gpio, 1);
+	udelay(1); /* Conversion time: 650 ns max */
+}
+
 static irqreturn_t ad7476_trigger_handler(int irq, void  *p)
 {
 	struct iio_poll_func *pf = p;
@@ -71,6 +85,8 @@ static irqreturn_t ad7476_trigger_handler(int irq, void  *p)
 	struct ad7476_state *st = iio_priv(indio_dev);
 	int b_sent;
 
+	ad7091_convst(st);
+
 	b_sent = spi_sync(st->spi, &st->msg);
 	if (b_sent < 0)
 		goto done;
@@ -254,6 +270,12 @@ static int ad7476_probe(struct spi_device *spi)
 	if (ret)
 		return ret;
 
+	st->convst_gpio = devm_gpiod_get_optional(&spi->dev,
+						  "adi,conversion-start",
+						  GPIOD_OUT_LOW);
+	if (IS_ERR(st->convst_gpio))
+		return PTR_ERR(st->convst_gpio);
+
 	spi_set_drvdata(spi, indio_dev);
 
 	st->spi = spi;
-- 
2.17.1

[PATCH 2/4] iio: adc: ad7476: Add IIO_CHAN_INFO_RAW for AD7091R

[Beniamin Bia]

From: Dragos Bogdan <dragos.bogdan@analog.com>

When CONVST signal is generated internally, IIO_CHAN_INFO_RAW can be
made available for AD7091R for single reads. This patch enables it and
makes supporting more devices by this driver easier.

Signed-off-by: Dragos Bogdan <dragos.bogdan@analog.com>
Signed-off-by: Beniamin Bia <beniamin.bia@analog.com>
---
 drivers/iio/adc/ad7476.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/iio/adc/ad7476.c b/drivers/iio/adc/ad7476.c
index 32e857dfec9c..3b48073dd62d 100644
--- a/drivers/iio/adc/ad7476.c
+++ b/drivers/iio/adc/ad7476.c
@@ -29,6 +29,8 @@ struct ad7476_state;
 struct ad7476_chip_info {
 	unsigned int			int_vref_uv;
 	struct iio_chan_spec		channel[2];
+	/* channels used when convst gpio is defined */
+	struct iio_chan_spec		convst_channel[2];
 	void (*reset)(struct ad7476_state *);
 };
 
@@ -109,6 +111,8 @@ static int ad7476_scan_direct(struct ad7476_state *st)
 {
 	int ret;
 
+	ad7091_convst(st);
+
 	ret = spi_sync(st->spi, &st->msg);
 	if (ret)
 		return ret;
@@ -176,6 +180,8 @@ static int ad7476_read_raw(struct iio_dev *indio_dev,
 #define AD7940_CHAN(bits) _AD7476_CHAN((bits), 15 - (bits), \
 		BIT(IIO_CHAN_INFO_RAW))
 #define AD7091R_CHAN(bits) _AD7476_CHAN((bits), 16 - (bits), 0)
+#define AD7091R_CONVST_CHAN(bits) _AD7476_CHAN((bits), 16 - (bits), \
+		BIT(IIO_CHAN_INFO_RAW))
 #define ADS786X_CHAN(bits) _AD7476_CHAN((bits), 12 - (bits), \
 		BIT(IIO_CHAN_INFO_RAW))
 
@@ -183,6 +189,8 @@ static const struct ad7476_chip_info ad7476_chip_info_tbl[] = {
 	[ID_AD7091R] = {
 		.channel[0] = AD7091R_CHAN(12),
 		.channel[1] = IIO_CHAN_SOFT_TIMESTAMP(1),
+		.convst_channel[0] = AD7091R_CONVST_CHAN(12),
+		.convst_channel[1] = IIO_CHAN_SOFT_TIMESTAMP(1),
 		.reset = ad7091_reset,
 	},
 	[ID_AD7276] = {
@@ -288,6 +296,9 @@ static int ad7476_probe(struct spi_device *spi)
 	indio_dev->channels = st->chip_info->channel;
 	indio_dev->num_channels = 2;
 	indio_dev->info = &ad7476_info;
+
+	if (st->convst_gpio && st->chip_info->convst_channel)
+		indio_dev->channels = st->chip_info->convst_channel;
 	/* Setup default message */
 
 	st->xfer.rx_buf = &st->data;
-- 
2.17.1

Re: [PATCH 2/4] iio: adc: ad7476: Add IIO_CHAN_INFO_RAW for AD7091R

[Jonathan Cameron]

On Wed, 11 Mar 2020 10:43:26 +0200
Beniamin Bia <beniamin.bia@analog.com> wrote:

> From: Dragos Bogdan <dragos.bogdan@analog.com>
> 
> When CONVST signal is generated internally, IIO_CHAN_INFO_RAW can be
> made available for AD7091R for single reads. This patch enables it and
> makes supporting more devices by this driver easier.
> 
> Signed-off-by: Dragos Bogdan <dragos.bogdan@analog.com>
> Signed-off-by: Beniamin Bia <beniamin.bia@analog.com>
Applied.

Thanks,

Jonathan

> ---
>  drivers/iio/adc/ad7476.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/iio/adc/ad7476.c b/drivers/iio/adc/ad7476.c
> index 32e857dfec9c..3b48073dd62d 100644
> --- a/drivers/iio/adc/ad7476.c
> +++ b/drivers/iio/adc/ad7476.c
> @@ -29,6 +29,8 @@ struct ad7476_state;
>  struct ad7476_chip_info {
>  	unsigned int			int_vref_uv;
>  	struct iio_chan_spec		channel[2];
> +	/* channels used when convst gpio is defined */
> +	struct iio_chan_spec		convst_channel[2];
>  	void (*reset)(struct ad7476_state *);
>  };
>  
> @@ -109,6 +111,8 @@ static int ad7476_scan_direct(struct ad7476_state *st)
>  {
>  	int ret;
>  
> +	ad7091_convst(st);
> +
>  	ret = spi_sync(st->spi, &st->msg);
>  	if (ret)
>  		return ret;
> @@ -176,6 +180,8 @@ static int ad7476_read_raw(struct iio_dev *indio_dev,
>  #define AD7940_CHAN(bits) _AD7476_CHAN((bits), 15 - (bits), \
>  		BIT(IIO_CHAN_INFO_RAW))
>  #define AD7091R_CHAN(bits) _AD7476_CHAN((bits), 16 - (bits), 0)
> +#define AD7091R_CONVST_CHAN(bits) _AD7476_CHAN((bits), 16 - (bits), \
> +		BIT(IIO_CHAN_INFO_RAW))
>  #define ADS786X_CHAN(bits) _AD7476_CHAN((bits), 12 - (bits), \
>  		BIT(IIO_CHAN_INFO_RAW))
>  
> @@ -183,6 +189,8 @@ static const struct ad7476_chip_info ad7476_chip_info_tbl[] = {
>  	[ID_AD7091R] = {
>  		.channel[0] = AD7091R_CHAN(12),
>  		.channel[1] = IIO_CHAN_SOFT_TIMESTAMP(1),
> +		.convst_channel[0] = AD7091R_CONVST_CHAN(12),
> +		.convst_channel[1] = IIO_CHAN_SOFT_TIMESTAMP(1),
>  		.reset = ad7091_reset,
>  	},
>  	[ID_AD7276] = {
> @@ -288,6 +296,9 @@ static int ad7476_probe(struct spi_device *spi)
>  	indio_dev->channels = st->chip_info->channel;
>  	indio_dev->num_channels = 2;
>  	indio_dev->info = &ad7476_info;
> +
> +	if (st->convst_gpio && st->chip_info->convst_channel)
> +		indio_dev->channels = st->chip_info->convst_channel;
>  	/* Setup default message */
>  
>  	st->xfer.rx_buf = &st->data;

[PATCH 3/4] iio: adc: ad7476: Add AD7091 support

[Beniamin Bia]

From: Dragos Bogdan <dragos.bogdan@analog.com>

AD7091R is already supported by this driver. While AD7091R allows the
choice of an internal or an external voltage reference, for AD7091 the
reference is only provided by VDD. Since this information is anyway
obtained through the "vcc" regulator, no other driver changes are
required for adding AD7091 support as well.

Signed-off-by: Dragos Bogdan <dragos.bogdan@analog.com>
Signed-off-by: Beniamin Bia <beniamin.bia@analog.com>
---
 drivers/iio/adc/ad7476.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/iio/adc/ad7476.c b/drivers/iio/adc/ad7476.c
index 3b48073dd62d..cd7d42df2033 100644
--- a/drivers/iio/adc/ad7476.c
+++ b/drivers/iio/adc/ad7476.c
@@ -341,6 +341,7 @@ static int ad7476_remove(struct spi_device *spi)
 }
 
 static const struct spi_device_id ad7476_id[] = {
+	{"ad7091", ID_AD7091R},
 	{"ad7091r", ID_AD7091R},
 	{"ad7273", ID_AD7277},
 	{"ad7274", ID_AD7276},
-- 
2.17.1

Re: [PATCH 3/4] iio: adc: ad7476: Add AD7091 support

[Jonathan Cameron]

On Wed, 11 Mar 2020 10:43:27 +0200
Beniamin Bia <beniamin.bia@analog.com> wrote:

> From: Dragos Bogdan <dragos.bogdan@analog.com>
> 
> AD7091R is already supported by this driver. While AD7091R allows the
> choice of an internal or an external voltage reference, for AD7091 the
> reference is only provided by VDD. Since this information is anyway
> obtained through the "vcc" regulator, no other driver changes are
> required for adding AD7091 support as well.
> 
> Signed-off-by: Dragos Bogdan <dragos.bogdan@analog.com>
> Signed-off-by: Beniamin Bia <beniamin.bia@analog.com>
Applied.

Thanks,

Jonathan

> ---
>  drivers/iio/adc/ad7476.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/iio/adc/ad7476.c b/drivers/iio/adc/ad7476.c
> index 3b48073dd62d..cd7d42df2033 100644
> --- a/drivers/iio/adc/ad7476.c
> +++ b/drivers/iio/adc/ad7476.c
> @@ -341,6 +341,7 @@ static int ad7476_remove(struct spi_device *spi)
>  }
>  
>  static const struct spi_device_id ad7476_id[] = {
> +	{"ad7091", ID_AD7091R},
>  	{"ad7091r", ID_AD7091R},
>  	{"ad7273", ID_AD7277},
>  	{"ad7274", ID_AD7276},

[PATCH 4/4] iio: adc: ad7476: implement devm_add_action_or_reset

[Beniamin Bia]

Use devm_add_action_or_reset to automatically disable the device
when it is removed or an error occurs during probe routine.

Signed-off-by: Beniamin Bia <beniamin.bia@analog.com>
---
 drivers/iio/adc/ad7476.c | 25 ++++++++++++-------------
 1 file changed, 12 insertions(+), 13 deletions(-)

diff --git a/drivers/iio/adc/ad7476.c b/drivers/iio/adc/ad7476.c
index cd7d42df2033..e9984a38fc4c 100644
--- a/drivers/iio/adc/ad7476.c
+++ b/drivers/iio/adc/ad7476.c
@@ -256,6 +256,13 @@ static const struct iio_info ad7476_info = {
 	.read_raw = &ad7476_read_raw,
 };
 
+static void ad7476_reg_disable(void *data)
+{
+	struct ad7476_state *st = data;
+
+	regulator_disable(st->reg);
+}
+
 static int ad7476_probe(struct spi_device *spi)
 {
 	struct ad7476_state *st;
@@ -278,6 +285,11 @@ static int ad7476_probe(struct spi_device *spi)
 	if (ret)
 		return ret;
 
+	ret = devm_add_action_or_reset(&spi->dev, ad7476_reg_disable,
+				       st);
+	if (ret)
+		return ret;
+
 	st->convst_gpio = devm_gpiod_get_optional(&spi->dev,
 						  "adi,conversion-start",
 						  GPIOD_OUT_LOW);
@@ -328,18 +340,6 @@ static int ad7476_probe(struct spi_device *spi)
 	return ret;
 }
 
-static int ad7476_remove(struct spi_device *spi)
-{
-	struct iio_dev *indio_dev = spi_get_drvdata(spi);
-	struct ad7476_state *st = iio_priv(indio_dev);
-
-	iio_device_unregister(indio_dev);
-	iio_triggered_buffer_cleanup(indio_dev);
-	regulator_disable(st->reg);
-
-	return 0;
-}
-
 static const struct spi_device_id ad7476_id[] = {
 	{"ad7091", ID_AD7091R},
 	{"ad7091r", ID_AD7091R},
@@ -377,7 +377,6 @@ static struct spi_driver ad7476_driver = {
 		.name	= "ad7476",
 	},
 	.probe		= ad7476_probe,
-	.remove		= ad7476_remove,
 	.id_table	= ad7476_id,
 };
 module_spi_driver(ad7476_driver);
-- 
2.17.1

Re: [PATCH 4/4] iio: adc: ad7476: implement devm_add_action_or_reset

[Jonathan Cameron]

On Wed, 11 Mar 2020 10:43:28 +0200
Beniamin Bia <beniamin.bia@analog.com> wrote:

> Use devm_add_action_or_reset to automatically disable the device
> when it is removed or an error occurs during probe routine.
> 
> Signed-off-by: Beniamin Bia <beniamin.bia@analog.com>
Applied,

Thanks,

Jonathan
> ---
>  drivers/iio/adc/ad7476.c | 25 ++++++++++++-------------
>  1 file changed, 12 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/iio/adc/ad7476.c b/drivers/iio/adc/ad7476.c
> index cd7d42df2033..e9984a38fc4c 100644
> --- a/drivers/iio/adc/ad7476.c
> +++ b/drivers/iio/adc/ad7476.c
> @@ -256,6 +256,13 @@ static const struct iio_info ad7476_info = {
>  	.read_raw = &ad7476_read_raw,
>  };
>  
> +static void ad7476_reg_disable(void *data)
> +{
> +	struct ad7476_state *st = data;
> +
> +	regulator_disable(st->reg);
> +}
> +
>  static int ad7476_probe(struct spi_device *spi)
>  {
>  	struct ad7476_state *st;
> @@ -278,6 +285,11 @@ static int ad7476_probe(struct spi_device *spi)
>  	if (ret)
>  		return ret;
>  
> +	ret = devm_add_action_or_reset(&spi->dev, ad7476_reg_disable,
> +				       st);
> +	if (ret)
> +		return ret;
> +
>  	st->convst_gpio = devm_gpiod_get_optional(&spi->dev,
>  						  "adi,conversion-start",
>  						  GPIOD_OUT_LOW);
> @@ -328,18 +340,6 @@ static int ad7476_probe(struct spi_device *spi)
>  	return ret;
>  }
>  
> -static int ad7476_remove(struct spi_device *spi)
> -{
> -	struct iio_dev *indio_dev = spi_get_drvdata(spi);
> -	struct ad7476_state *st = iio_priv(indio_dev);
> -
> -	iio_device_unregister(indio_dev);
> -	iio_triggered_buffer_cleanup(indio_dev);
> -	regulator_disable(st->reg);
> -
> -	return 0;
> -}
> -
>  static const struct spi_device_id ad7476_id[] = {
>  	{"ad7091", ID_AD7091R},
>  	{"ad7091r", ID_AD7091R},
> @@ -377,7 +377,6 @@ static struct spi_driver ad7476_driver = {
>  		.name	= "ad7476",
>  	},
>  	.probe		= ad7476_probe,
> -	.remove		= ad7476_remove,
>  	.id_table	= ad7476_id,
>  };
>  module_spi_driver(ad7476_driver);

Re: [PATCH 1/4] iio: adc: ad7476: Generate CONVST signal internally

[Jonathan Cameron]

On Wed, 11 Mar 2020 10:43:25 +0200
Beniamin Bia <beniamin.bia@analog.com> wrote:

> From: Dragos Bogdan <dragos.bogdan@analog.com>
> 
> Compared to the other supported parts, AD7091R are dependent of
> a CONVST signal that initiates the conversion. At this moment, only
> sampling in buffered mode is supported for AD7091R and the only
> option until now was to generate this signal externally using an
> IIO trigger. This patch adds the option of generating it internally,
> more compatible triggers being available in this case.
> 
> Also, it is an intermiate step of adding support more devices.
> 
> Signed-off-by: Dragos Bogdan <dragos.bogdan@analog.com>
> Signed-off-by: Beniamin Bia <beniamin.bia@analog.com>
Seems sensible.  Applied to the togreg branch of iio.git and pushed
out as testing for the autobuilders to play with it.

Thanks,

Jonathan

> ---
>  drivers/iio/adc/ad7476.c | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/drivers/iio/adc/ad7476.c b/drivers/iio/adc/ad7476.c
> index 76747488044b..32e857dfec9c 100644
> --- a/drivers/iio/adc/ad7476.c
> +++ b/drivers/iio/adc/ad7476.c
> @@ -12,9 +12,11 @@
>  #include <linux/sysfs.h>
>  #include <linux/spi/spi.h>
>  #include <linux/regulator/consumer.h>
> +#include <linux/gpio/consumer.h>
>  #include <linux/err.h>
>  #include <linux/module.h>
>  #include <linux/bitops.h>
> +#include <linux/delay.h>
>  
>  #include <linux/iio/iio.h>
>  #include <linux/iio/sysfs.h>
> @@ -34,6 +36,7 @@ struct ad7476_state {
>  	struct spi_device		*spi;
>  	const struct ad7476_chip_info	*chip_info;
>  	struct regulator		*reg;
> +	struct gpio_desc		*convst_gpio;
>  	struct spi_transfer		xfer;
>  	struct spi_message		msg;
>  	/*
> @@ -64,6 +67,17 @@ enum ad7476_supported_device_ids {
>  	ID_ADS7868,
>  };
>  
> +static void ad7091_convst(struct ad7476_state *st)
> +{
> +	if (!st->convst_gpio)
> +		return;
> +
> +	gpiod_set_value(st->convst_gpio, 0);
> +	udelay(1); /* CONVST pulse width: 10 ns min */
> +	gpiod_set_value(st->convst_gpio, 1);
> +	udelay(1); /* Conversion time: 650 ns max */
> +}
> +
>  static irqreturn_t ad7476_trigger_handler(int irq, void  *p)
>  {
>  	struct iio_poll_func *pf = p;
> @@ -71,6 +85,8 @@ static irqreturn_t ad7476_trigger_handler(int irq, void  *p)
>  	struct ad7476_state *st = iio_priv(indio_dev);
>  	int b_sent;
>  
> +	ad7091_convst(st);
> +
>  	b_sent = spi_sync(st->spi, &st->msg);
>  	if (b_sent < 0)
>  		goto done;
> @@ -254,6 +270,12 @@ static int ad7476_probe(struct spi_device *spi)
>  	if (ret)
>  		return ret;
>  
> +	st->convst_gpio = devm_gpiod_get_optional(&spi->dev,
> +						  "adi,conversion-start",
> +						  GPIOD_OUT_LOW);
> +	if (IS_ERR(st->convst_gpio))
> +		return PTR_ERR(st->convst_gpio);
> +
>  	spi_set_drvdata(spi, indio_dev);
>  
>  	st->spi = spi;